Bug 135399

Summary: Web Inspector: 80% of time during recording is spent creating source code locations for profile nodes
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2014-07-29 15:43:16 PDT
* SUMMARY
Recording a timeline of the inspector recording a timeline of logging into a website, 80% of the work Web Inspector was doing was creating source code location objects for ProfileNodes that were not even visible in the timeline overview. These source code locations should not be computed eagerly. especially because there are so many of them (1 per function call).

* STEPS TO REPRODUCE
1. Inspect iCloud.com
2. In inspector¹ select a large range in the timeline (0-20s)
3. Inspect inspector¹
4. Start a timeline in inspector²
5. Reload the page
  => very long time and hangs
Comment 1 Radar WebKit Bug Importer 2014-07-29 15:44:22 PDT
<rdar://problem/17848962>
Comment 2 Joseph Pecoraro 2014-07-29 21:13:36 PDT
Created attachment 235732 [details]
[PATCH] Proposed Fix

I attempted to merge LazySourceCodeLocation into SourceCodeLocation, but I wasn't see-ing the same performance wins as this patch, so I decided to go back to this. Ultimately I think it would make sense to fold this into SourceCodeLocation if it is possible. For now, I think this is a good compromise.
Comment 3 Timothy Hatcher 2014-07-29 22:10:50 PDT
Comment on attachment 235732 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=235732&action=review

> Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js:281
> +    _initializeProfileFromPayload: function(payload)

As much as I hate leaking protocol data past the controllers into the model, I this this is fine for the perf win.
Comment 4 WebKit Commit Bot 2014-07-29 22:42:38 PDT
Comment on attachment 235732 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 235732

Committed r171790: <http://trac.webkit.org/changeset/171790>
Comment 5 WebKit Commit Bot 2014-07-29 22:42:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Joseph Pecoraro 2014-07-30 10:50:00 PDT
(In reply to comment #3)
> (From update of attachment 235732 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235732&action=review
> 
> > Source/WebInspectorUI/UserInterface/Models/ScriptTimelineRecord.js:281
> > +    _initializeProfileFromPayload: function(payload)
> 
> As much as I hate leaking protocol data past the controllers into the model, I this this is fine for the perf win.

The profileFromPayload could go anywhere (it could go in TimelineManager). It is a standalone factory like method.