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+

Kent Tamura
Reported 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
Attachments
Patch (3.43 KB, patch)
2012-05-21 03:18 PDT, Kent Tamura
no flags
Patch 2 (3.41 KB, patch)
2012-05-21 17:35 PDT, Kent Tamura
rniwa: review+
Kent Tamura
Comment 1 2012-05-21 03:18:02 PDT
Oh, "webkit-patch upload" doesn't work for the original bug tittle :-P
Kent Tamura
Comment 2 2012-05-21 03:18:37 PDT
Kent Tamura
Comment 3 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?
Ryosuke Niwa
Comment 4 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.
Eric Seidel (no email)
Comment 5 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?
Kent Tamura
Comment 6 2012-05-21 17:35:09 PDT
Kent Tamura
Comment 7 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.
Ryosuke Niwa
Comment 8 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.
Kent Tamura
Comment 9 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
Ryosuke Niwa
Comment 10 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?
Kent Tamura
Comment 11 2012-05-21 17:52:56 PDT
Kent Tamura
Comment 12 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.
Note You need to log in before you can comment on or make changes to this bug.