Bug 136796

<rdar://problem/18327309>

Created attachment 238091 [details] Patch

Attachment 238091 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/profiler/ProfileGenerator.cpp:51: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 238091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238091&action=review > Source/JavaScriptCore/ChangeLog:13 > + Time for a node may elapse in a non-contiguous fashion depending on the interleaving of > + didPause, didContinue, willExecute, and didExecute. A node's start time is set to the > + start of the last such interval that accrues elapsed time. This sounds like it could be confusing in the Timeline data grid. Maybe once TimelineRecords account for pausing it will be fine? Why does start time need to move? If the profile data was visualized, I'd expect to see start time stay put and end time stay put, but there would be gaps where the debugger paused and resumed. That would require more complex data in ProfileNode::Call.

Comment on attachment 238091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238091&action=review >> Source/JavaScriptCore/ChangeLog:13 >> + start of the last such interval that accrues elapsed time. > > This sounds like it could be confusing in the Timeline data grid. Maybe once TimelineRecords account for pausing it will be fine? Why does start time need to move? > > If the profile data was visualized, I'd expect to see start time stay put and end time stay put, but there would be gaps where the debugger paused and resumed. That would require more complex data in ProfileNode::Call. The "more complex data" likely just needs to be a vector of debugger pause and resume timestamp pairs. It could even be global to the profile and the front end uses it when displaying the data or visuals.

(In reply to comment #4) > (From update of attachment 238091 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=238091&action=review > > > Source/JavaScriptCore/ChangeLog:13 > > + Time for a node may elapse in a non-contiguous fashion depending on the interleaving of > > + didPause, didContinue, willExecute, and didExecute. A node's start time is set to the > > + start of the last such interval that accrues elapsed time. > > This sounds like it could be confusing in the Timeline data grid. Maybe once TimelineRecords account for pausing it will be fine? Why does start time need to move? > > If the profile data was visualized, I'd expect to see start time stay put and end time stay put, but there would be gaps where the debugger paused and resumed. That would require more complex data in ProfileNode::Call. I think it's worth clarifying: these proposed changes (and others for Timeline records) are moving away from wall-clock time and towards elapsed execution time (of which, time paused in the debugger does not count towards). IMO, the current wall-clock approach---including paused time in profile nodes and timeline records---is almost always going to make the timeline overview unusable. And for replay recordings, a user will probably pause the debugger at different places on different playbacks. It would be nice if the timeline looked more or less the same for all playbacks, otherwise it's not a very good navigation tool.

Committed r173939: <http://trac.webkit.org/changeset/173939>