Bug 86994 - Add performance tests for textarea
Summary: Add performance tests for textarea
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks: 86813
  Show dependency treegraph
 
Reported: 2012-05-21 00:01 PDT by Kent Tamura
Modified: 2012-05-21 17:55 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.43 KB, patch)
2012-05-21 03:18 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (3.41 KB, patch)
2012-05-21 17:35 PDT, Kent Tamura
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.