|
Re: Eclipselink PerformanceProfiler Not Thread Safe [message #1764967 is a reply to message #1764950] |
Mon, 05 June 2017 08:36 |
Nuno Godinho de Matos Messages: 34 Registered: September 2012 |
Member |
|
|
I have another question to add to the above.
Is the Performance profiler not becoming a complete memory leak in the system?
Based on a heap dump I have just taken, the PerformanceProfiler is accumulating thousands of Profiles over time, and each profile has potentially multiple OperatingTimings in there.
E.g. one operation timing per type of operation... parse sql, transaction commit, etc...
Over time the number of profiles sky rockets.
Once could argue, that if the performance profiler were properly implemented.
Then the number of Profiles would stop growing over time.
Why?
Because the different operations supported by the application would eventually all get a Profile into a Map.
All entities covered on all operation types.
And the PerformanceProfile would be managing its list of profiles with some sort of Map OpertionTypexClass -> Profile aggregating time of all queries.
But this seems not to be the case.
So from the looks of it we have the:
1. public void endOperationProfile(String operationName) {
That does an unconditional addProfile. Does not check for any map to pump aggregated data.
So this is memory leaker number one.
2. We have public Object profileExecutionOfQuery(DatabaseQuery query, Record row, AbstractSession session) {
This is leaker number two.
Also uncoditionally adds a new profile for each query run.
Am I missing something, or is this code completely out of this world?
If I run system tests for 5 minutes with 1.5 GB heap my system is running out of memory.
Whatever burns you first, either cncurrency issues trying to get the profiling summary killing transactions.
Or the number of profiles in the system sky rocketing and the GC log gobbing all CPU trying to free up the un-freable, because the serverssion profiler is not going anywhere.
Am I missing something from the picture, or this Performance profiler completely unusable?
How can it be written in this manner?
From what I can see, the core idea is very good, but the implementation behind this is so terrible, that the full class needs to be re-written from scrath. This is my current impression.
This performance profiler, since it is keeping the data in memory, it clarly needs an Application Scoped state that will not grow to infinity.
The class needs proper comprhensible map map of OperationTypeKey -> To Aggregated statistics of total time.
And for each Value taken out of the map, there should be a simple operation to:
Merge Timings from current operation into Aggregated timings in this app scoped map.
Then the PerformanceProfiler would be only as big as the number of different profiles you need to qualif the ClassType -> Performance.
Or OperationType (e.g ReadAll, FetchById, et) -> Performance.
Finally, for the interested consumers, the performance profile should provide adequate publich apis to return summary as String or as metadata.
Not this thing of simply assuming we all want to write the summary to the session.Writer().
Many thanks for taking a look.
In my oppinion this issue is critical - because this is an analysis tool that appears to be there, but it is really not there. You will end up analysing the side effects of using this tool rather than the performanc of your system.
[Updated on: Mon, 05 June 2017 08:58] Report message to a moderator
|
|
|
|
Powered by
FUDForum. Page generated in 0.02247 seconds