Bug 31291 - Web Inspector: Speed up syntax highlighter
: Web Inspector: Speed up syntax highlighter
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-10 06:30 PST by Keishi Hattori
Modified: 2009-11-10 13:04 PST (History)
8 users (show)

See Also:


Attachments
proposed patch (18.11 KB, patch)
2009-11-10 07:52 PST, Keishi Hattori
timothy: review-
Details | Formatted Diff | Diff
proposed patch 2 (18.11 KB, patch)
2009-11-10 09:59 PST, Keishi Hattori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2009-11-10 06:30:45 PST
I discovered that DocumentFragment is much slower than using Node when it has many childNodes.
Comment 1 Keishi Hattori 2009-11-10 07:52:46 PST
Created attachment 42865 [details]
proposed patch

Removed use of DocumentFragment. Chromium scrolls like butter even while syntax highlighting huge minified files. 

WebKit Nightly is not much faster. The DOM performance issue occurs for Node.appendChild(Node) too.
https://bugs.webkit.org/show_bug.cgi?id=31237
We can bump up the line length limit when this is resolved.
Comment 2 Timothy Hatcher 2009-11-10 09:26:36 PST
Comment on attachment 42865 [details]
proposed patch

> -                while (node.firstChild)
> -                    node.removeChild(node.firstChild);

Why isn't this needed in the loop anymore? (I see removeChildren was added outside the loop.)


> -                this.lineFragment =null;
> +                this.newLine =null;

Add a space before null.


> -        action: identOrKeywordAction,
> -        dontAppendNonToken: true
> +        action: identOrKeywordAction

Why this change?

r- for the missing space.
Comment 3 Timothy Hatcher 2009-11-10 09:26:57 PST
Did you file WebCore bugs about the slow downs?
Comment 4 Keishi Hattori 2009-11-10 09:59:42 PST
Created attachment 42871 [details]
proposed patch 2

(In reply to comment #2)
> (From update of attachment 42865 [details])
> > -                while (node.firstChild)
> > -                    node.removeChild(node.firstChild);
> 
> Why isn't this needed in the loop anymore? (I see removeChildren was added
> outside the loop.)

I removeChildren before the loop and use that directly for this.newLine, eliminating the need for a intermediate node.

> > -                this.lineFragment =null;
> > +                this.newLine =null;
> 
> Add a space before null.

Fixed.

> > -        action: identOrKeywordAction,
> > -        dontAppendNonToken: true
> > +        action: identOrKeywordAction
> 
> Why this change?

I fixed a mistake here. 
I was going to treat ident as a non-token so dontAppendNonToken was set to true to cut down on the number of TextNodes. 
However the current code treats both ident and keyword as proper tokens. So I removed the two "this.appendNonToken();"  from identOrKeywordAction and set dontAppendNonToken to false so this.appendNonToken will get called inside lex().

> Did you file WebCore bugs about the slow downs?

I filed Bug 31237. I'll try to add some more data and a reduced test case.
Comment 5 WebKit Commit Bot 2009-11-10 13:04:14 PST
Comment on attachment 42871 [details]
proposed patch 2

Clearing flags on attachment: 42871

Committed r50755: <http://trac.webkit.org/changeset/50755>
Comment 6 WebKit Commit Bot 2009-11-10 13:04:19 PST
All reviewed patches have been landed.  Closing bug.