[webkit-reviews] review requested: [Bug 31291] Web Inspector: Speed up syntax highlighter : [Attachment 42871] proposed patch 2

[bugzilla-daemon at webkit.org]

Keishi Hattori <casey.hattori at gmail.com> has asked  for review:
Bug 31291: Web Inspector: Speed up syntax highlighter
https://bugs.webkit.org/show_bug.cgi?id=31291

Attachment 42871: proposed patch 2
https://bugs.webkit.org/attachment.cgi?id=42871&action=review

------- Additional Comments from Keishi Hattori <casey.hattori at gmail.com>
(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.