RESOLVED INVALID 26418
Profiler 'Call' count is incorrect in Heavy view
https://bugs.webkit.org/show_bug.cgi?id=26418
Summary Profiler 'Call' count is incorrect in Heavy view
Kevin McCullough
Reported 2009-06-15 15:02:31 PDT
6/12/09 12:09 PM Kevin McCullough: * SUMMARY In Heavy view callers have the same number of calls as their root, or called, functions. This is not correct and does not show up in the Tree view. * STEPS TO REPRODUCE 1. Load the attached test page in Safari 2. Start the profiler 3. Click the run-test button 4. Stop the profiler 5. Look at the heavy and tree views and see how the number of calls assigned to callers is different (e.g. 'runTest' was only called once, but in heavy view it says it was called 3 times <rdar://problem/6968125> It turns out that call, self and total time are all incorrect for callers in Heavy View
Attachments
Test (653 bytes, text/html)
2009-06-15 15:03 PDT, Kevin McCullough
no flags
Screenshot of error (61.41 KB, image/png)
2009-06-15 15:26 PDT, Kevin McCullough
no flags
proposed patch (1.90 KB, patch)
2009-06-15 15:27 PDT, Kevin McCullough
timothy: review+
Correct behavior (61.37 KB, image/png)
2009-06-15 15:37 PDT, Kevin McCullough
no flags
Kevin McCullough
Comment 1 2009-06-15 15:03:47 PDT
Kevin McCullough
Comment 2 2009-06-15 15:26:19 PDT
Created attachment 31312 [details] Screenshot of error Notice that 'runTest' is listed 3 times in heavy view but all three show different %s and times.
Kevin McCullough
Comment 3 2009-06-15 15:27:28 PDT
Created attachment 31313 [details] proposed patch
Timothy Hatcher
Comment 4 2009-06-15 15:34:13 PDT
Can you post a screenshot of the new behaviour?
Kevin McCullough
Comment 5 2009-06-15 15:37:18 PDT
Created attachment 31314 [details] Correct behavior
Kevin McCullough
Comment 6 2009-06-15 15:39:55 PDT
Committed revision 44697.
Francisco Tolmasky
Comment 7 2009-06-15 15:47:41 PDT
The original behavior was correct. The calls are supposed to be different than tree view. There is an in depth explanation as to what they mean here: http://www.alertdebugging.com/2009/04/29/building-a-better-javascript-profiler-with-webkit/ The purpose of the calls/percentages/etc is to show a reverse backtrace not to simply "flip tree view on its head". The way to read the original profile, showing "3" in all three columns, is as follows: "three calls to log to place from runTest", NOT "three runTests resulted in 3 logs". Had you had multiple functions calling "console.log" you would see how they break down. This is how it works in Shark, and the purpose is to be able to identify where most of the "offending" calls are coming from. Again, if you like I can list this here but the link above does a good job of explaining it.
Francisco Tolmasky
Comment 8 2009-06-15 15:48:28 PDT
Sorry, that should read "three calls to log took place from runTest" (In reply to comment #7) > The original behavior was correct. The calls are supposed to be different than > tree view. There is an in depth explanation as to what they mean here: > http://www.alertdebugging.com/2009/04/29/building-a-better-javascript-profiler-with-webkit/ > > The purpose of the calls/percentages/etc is to show a reverse backtrace not to > simply "flip tree view on its head". > > The way to read the original profile, showing "3" in all three columns, is as > follows: "three calls to log to place from runTest", NOT "three runTests > resulted in 3 logs". Had you had multiple functions calling "console.log" you > would see how they break down. This is how it works in Shark, and the purpose > is to be able to identify where most of the "offending" calls are coming from. > Again, if you like I can list this here but the link above does a good job of > explaining it. >
Adam Roben (:aroben)
Comment 9 2009-06-15 15:52:07 PDT
Comment on attachment 31313 [details] proposed patch > + <rdar://problem/6968125> Profiler 'Call' count is incorrect in Heavy > + view (26418) We normally include the Bugzilla URLs in the ChangeLog, as well.
Mark Rowe (bdash)
Comment 10 2009-06-15 22:25:55 PDT
Reopening as this was rolled out due to being incorrect.
Mark Rowe (bdash)
Comment 11 2009-06-15 22:26:35 PDT
It was rolled out in: <http://trac.webkit.org/changeset/44698>. Closing as INVALID to reflect that the behavior was correct.
Note You need to log in before you can comment on or make changes to this bug.