Community
Participate
Working Groups
In certain cases, execution statistics seems to be repeating the same set of profiling statistics over-and-over again, independent of what the target application is actually doing. To reproduce: 1. From Eclipse, profile HeapJniCarTest with Execution Stats enabled. Set the polling frequency to 10 seconds before launching. 2. Launch the app. Hit enter once in the console, to advance the application to the second set of outputs. 3. Wait for stats to appear in the profiling window. Then, wait for the stats to have greater-than-zero method calls associated with them. After step 3, every subsequent data refresh will increase the time and call numbers on all the methods. This is inaccurate, because at this stage the profiled application is literally doing nothing. Each data refresh is essentially a copy of the last data refresh that was sent.
Created attachment 188203 [details] Patch to tickets.h/tickets.cpp
Mike, can you review?
I'd say this looks good overall; nicely cleaned up from the prior version :) A few minor comments/questions: - The comment block talking about 'Print Stack' (Tickets.h:193) would be more clear if you use 'visit' in place of 'enter'; this reduces confusion between the idea of a stack entry and the act of traversing the tree. - Can CStackEntry::m_pInvokedChildren be an aggregated member, rather than a pointer that has to b new'd? This avoids a call to new+delete at each level of the call-graph. - Similarly for CStackHead::m_pStackHead? - Tickets.cpp:53 - Does it make sense to emit a DEBUG message here? That way if this does happen we have a way of detecting it? - CStackEntry::DeleteTreeNonMatchingStackEntry - does the fact that you sometimes delete the entry prevent you from putting the increment operation in the third slot of the for loop, as is usual? Otherwise I would suggest moving the 'it++' to the third clause of the for loop, as it makes it easier to see there is not an infinite loop.
Created attachment 188259 [details] Patch to tickets.h/tickets.cpp - 2
I would like to request that the following defect be considered for PMC approval for 4.7.2. 1. Explain why you believe this is a stop-ship defect. How does the defect manifest itself, and how will users of TPTP / consuming products be affected if the defect is not fixed? The scope of the issue described above is such that it is likely that incorrect statistics would be produced in any profile with execution statistics, and should therefore be fixed in 4.7.2. 2. Is there a work-around? If so, why do you believe the work-around is insufficient? Workaround would be to used execution flow, rather than execution statistics, however this is not an option for profiling large or long-running applications, or applications for which only minimal profiling overhead can be tolerated. 3. Is this a regression or API breakage? Explain. No, not a regression from 4.7.1, haven't tested issue with other previous releases. 4. Does this require new API? No. 5. Who performed the code review? Mike Reid. 6. Is there a test case attached to the bugzilla record? No. 7. What is the nature of the fix? What is the risk associated with this fix? Fix required rewriting the algorithm used to process execution statistics data. New algorithm has been fully documented in the code. This change does not affect profiling with 'execution flow' option, which is the default for execution profiling. Risk is medium. 8. Is this fix related to any standards that TPTP adheres to? If so, who has validated that the fix continues to adhere to the standard? No.
Requesting PMC approval for TPTP 4.7.2.
I've tested the patch in a variety of scenarios, and on a number of large application configurations, and have run the automated JVMTI test on the change and had no issues with any of the above. Once a build is available I will verify the fix through additional testing.
Checked into HEAD w/ PMC approval.
Hey Jon, the build failed on the patch you checked in. please handle Thanks, Sam
Created attachment 188372 [details] Patch to tickets.h/tickets.cpp - 3
Patch 2 compiled fine for me on VS2K3, but this should fix for the rest of the platforms.
Checked into HEAD.
Closing.
