Bug 135399 - Web Inspector: 80% of time during recording is spent creating source code locations for profile nodes
Summary: Web Inspector: 80% of time during recording is spent creating source code loc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-07-29 15:43 PDT by Joseph Pecoraro
Modified: 2014-07-30 10:50 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (34.10 KB, patch)
2014-07-29 21:13 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.