Bug 34921 - Web Inspector: cache created row, span and text elements in TextView to improve scroller performance on debug build.
Summary: Web Inspector: cache created row, span and text elements in TextView to impro...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-13 07:29 PST by Pavel Feldman
Modified: 2010-02-14 01:21 PST (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed change. (12.87 KB, patch)
2010-02-13 07:36 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-02-13 07:29:32 PST
Release mode is fast both with and without this change. I noticed that our styles are taking too long to recalc though (due to selector complexity?)
Comment 1 Pavel Feldman 2010-02-13 07:36:51 PST
Created attachment 48704 [details]
[PATCH] Proposed change.
Comment 2 Timothy Hatcher 2010-02-13 07:57:06 PST
Comment on attachment 48704 [details]
[PATCH] Proposed change.


> +            var span = this._cachedSpans[this._cachedSpans.length - 1];
> +            this._cachedSpans.length--;

I am surprised this works! You should just use the pop() function. This is done in a few other places.


I also don't understand how this helps much, since you just cache te elements, but the contents are replaces every time.

r+ if you change to use pop() before landing.
Comment 3 Pavel Feldman 2010-02-14 01:21:12 PST
(In reply to comment #2)
> (From update of attachment 48704 [details])
> 
> > +            var span = this._cachedSpans[this._cachedSpans.length - 1];
> > +            this._cachedSpans.length--;
> 
> I am surprised this works! You should just use the pop() function. This is done
> in a few other places.
> 
> 
> I also don't understand how this helps much, since you just cache te elements,
> but the contents are replaces every time.

Scrolling a large document slowly needs thousands of spans to be created at runtime for the highlight, thousands of rows to be created for lines. Native elements are allocated in heap, js wrappers are created for all of them since they are used in our code. Caching allows to limit number of created spans to a couple of hundreds.

> 
> r+ if you change to use pop() before landing.

Done.


Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/SourceFrame.js
	M	WebCore/inspector/front-end/TextViewer.js
Committed r54756