Bug 31291 - Web Inspector: Speed up syntax highlighter
: Web Inspector: Speed up syntax highlighter
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-11-10 06:30 PST by
Modified: 2009-11-10 13:04 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-11-10 06:30:45 PST
I discovered that DocumentFragment is much slower than using Node when it has many childNodes.
------- Comment #1 From 2009-11-10 07:52:46 PST -------
Created an attachment (id=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 From 2009-11-10 09:26:36 PST -------
(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.)


> -                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 From 2009-11-10 09:26:57 PST -------
Did you file WebCore bugs about the slow downs?
------- Comment #4 From 2009-11-10 09:59:42 PST -------
Created an attachment (id=42871) [details]
proposed patch 2

(In reply to comment #2)
> (From update of attachment 42865 [details] [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 From 2009-11-10 13:04:14 PST -------
(From update of attachment 42871 [details])
Clearing flags on attachment: 42871

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