Bug 31291

Summary: Web Inspector: Speed up syntax highlighter
Product: WebKit Reporter: Keishi Hattori <keishi>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
proposed patch
timothy: review-
proposed patch 2 none

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.