Bug 86994

Summary: Add performance tests for textarea
Product: WebKit Reporter: Kent Tamura <tkent>
Component: Tools / TestsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: haraken, morrita, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 86813    
Attachments:
Description Flags
Patch
none
Patch 2 rniwa: review+

Description Kent Tamura 2012-05-21 00:01:05 PDT
Add some performance tests to prepare to fix Bug 86813 and http://code.google.com/p/chromium/issues/detail?id=109587
Comment 1 Kent Tamura 2012-05-21 03:18:02 PDT
Oh, "webkit-patch upload" doesn't work for the original bug tittle :-P
Comment 2 Kent Tamura 2012-05-21 03:18:37 PDT
Created attachment 142981 [details]
Patch
Comment 3 Kent Tamura 2012-05-21 03:22:06 PDT
(In reply to comment #2)
> Created an attachment (id=142981) [details]
> Patch

On my machine, these tests take the following time:

RESULT DOM: textarea-dom= 1249.8 ms
median= 1249.0 ms, stdev= 2.85657137142 ms, min= 1245.0 ms, max= 1257.0 ms

RESULT DOM: textarea-edit= 1691.15 ms
median= 1693.5 ms, stdev= 10.7576716812 ms, min= 1669.0 ms, max= 1707.0 ms

RESULT Parser: textarea-parsing= 880.95 ms
median= 877.0 ms, stdev= 29.0714206739 ms, min= 842.0 ms, max= 959.0 ms

Are they too long? or too short?
Comment 4 Ryosuke Niwa 2012-05-21 07:39:52 PDT
(In reply to comment #3)
> RESULT DOM: textarea-dom= 1249.8 ms
> median= 1249.0 ms, stdev= 2.85657137142 ms, min= 1245.0 ms, max= 1257.0 ms
> 
> RESULT DOM: textarea-edit= 1691.15 ms
> median= 1693.5 ms, stdev= 10.7576716812 ms, min= 1669.0 ms, max= 1707.0 ms
> 
> RESULT Parser: textarea-parsing= 880.95 ms
> median= 877.0 ms, stdev= 29.0714206739 ms, min= 842.0 ms, max= 959.0 ms
> 
> Are they too long? or too short?

You can use runPerSecond to avoid making the test too slow or too fast. It'll automatically adjust the number of function calls to measure runs/s accurately.
Comment 5 Eric Seidel (no email) 2012-05-21 14:27:58 PDT
Comment on attachment 142981 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142981&action=review

> PerformanceTests/DOM/textarea-dom.html:19
> +        container.appendChild(document.createTextNode('A quick brown fox jumps over the lazy dog.\n'));
> +        container.appendChild(document.createTextNode('A quick brown fox jumps over the lazy dog.\n'));
> +        container.appendChild(document.createComment(' comment '));

Are you trying to test comment creation/textNode creation?  Do you want to pre-allocate those in an Array up front?
Comment 6 Kent Tamura 2012-05-21 17:35:09 PDT
Created attachment 143149 [details]
Patch 2
Comment 7 Kent Tamura 2012-05-21 17:36:53 PDT
(In reply to comment #4)
> > Are they too long? or too short?
> 
> You can use runPerSecond to avoid making the test too slow or too fast. It'll automatically adjust the number of function calls to measure runs/s accurately.

Thanks.  The new patch switched to runPerSecond.

(In reply to comment #5)
> (From update of attachment 142981 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142981&action=review
> 
> > PerformanceTests/DOM/textarea-dom.html:19
> > +        container.appendChild(document.createTextNode('A quick brown fox jumps over the lazy dog.\n'));
> > +        container.appendChild(document.createTextNode('A quick brown fox jumps over the lazy dog.\n'));
> > +        container.appendChild(document.createComment(' comment '));
> 
> Are you trying to test comment creation/textNode creation?  Do you want to pre-allocate those in an Array up front?

Good point.  I changed the test so that it uses pre-allocated nodes.
Comment 8 Ryosuke Niwa 2012-05-21 17:37:32 PDT
Comment on attachment 143149 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=143149&action=review

> PerformanceTests/DOM/textarea-dom.html:16
> +var ChildCount = 1000;

Use camelCase instead?

> PerformanceTests/Parser/textarea-parsing.html:18
> +    htmlText += "A quick brown fox jumps over the lazy dog.\n" +
> +        "A quick brown fox jumps over the lazy dog.\n" +
> +        "<!-- comment -->\n";

+ should appear at the beginning of the next line.
Comment 9 Kent Tamura 2012-05-21 17:45:34 PDT
Comment on attachment 143149 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=143149&action=review

>> PerformanceTests/Parser/textarea-parsing.html:18
>> +        "<!-- comment -->\n";
> 
> + should appear at the beginning of the next line.

Putting '+' at the end of lines is a good practice to avoid implicit semicolons.

http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?showone=Code_formatting#Code_formatting
Comment 10 Ryosuke Niwa 2012-05-21 17:47:31 PDT
Comment on attachment 143149 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=143149&action=review

>>> PerformanceTests/Parser/textarea-parsing.html:18
>>> +        "<!-- comment -->\n";
>> 
>> + should appear at the beginning of the next line.
> 
> Putting '+' at the end of lines is a good practice to avoid implicit semicolons.
> 
> http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?showone=Code_formatting#Code_formatting

I was thinking of http://www.webkit.org/coding/coding-style.html#indentation-wrap-bool-op but I guess they only apply to boolean operators?
Comment 11 Kent Tamura 2012-05-21 17:52:56 PDT
Committed r117862: <http://trac.webkit.org/changeset/117862>
Comment 12 Kent Tamura 2012-05-21 17:55:00 PDT
(In reply to comment #10)
> > Putting '+' at the end of lines is a good practice to avoid implicit semicolons.
> > 
> > http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?showone=Code_formatting#Code_formatting
> 
> I was thinking of http://www.webkit.org/coding/coding-style.html#indentation-wrap-bool-op but I guess they only apply to boolean operators?

I think we're following this rule for any operators in C++. But JavaScript needs a special treatment at this point.