Web Inspector: subtract elapsed time while debugger is paused from profile nodes

• Component: Web Inspector
• Status: RESOLVED FIXED

[Brian Burg]

Currently it will zero out any ProfileNode::Call where the debugger paused further down in the call stack. Instead, we should subtract the elapsed paused time.

[Radar WebKit Bug Importer]

<rdar://problem/18327309>

[Brian Burg]

Created attachment 238091 [details]
Patch

[WebKit Commit Bot]

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.

[Timothy Hatcher]

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.

[Timothy Hatcher]

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.

[Brian Burg]

(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.

[Brian Burg]

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