Bug 86017

Summary: add some basic perf-o-matic tests for flexbox
Product: WebKit Reporter: Tony Chang <tony>
Component: New BugsAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: ojan, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Tony Chang 2012-05-09 14:21:06 PDT
add some basic perf-o-matic tests for flexbox
Comment 1 Tony Chang 2012-05-09 14:23:10 PDT
Created attachment 141012 [details]
Patch
Comment 2 Tony Chang 2012-05-09 14:24:23 PDT
Comment on attachment 141012 [details]
Patch

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

> PerformanceTests/Layout/flexbox-column-nowrap.html:33
> +PerfTestRunner.run(runTest, 180, 20, function() {
> +    document.getElementById("flexbox").style.display = 'none';
> +});

Do I need the done function?  Without it, the column tests failed, but the row tests passed.
Comment 3 Ryosuke Niwa 2012-05-09 14:37:46 PDT
Comment on attachment 141012 [details]
Patch

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

> PerformanceTests/Layout/flexbox-column-nowrap.html:31
> +PerfTestRunner.run(runTest, 180, 20, function() {

It's fragile to rely on hard-coded values like this.

>> PerformanceTests/Layout/flexbox-column-nowrap.html:33
>> +});
> 
> Do I need the done function?  Without it, the column tests failed, but the row tests passed.

done is called after all tests have ran.
Comment 4 Tony Chang 2012-05-09 14:39:23 PDT
Comment on attachment 141012 [details]
Patch

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

>> PerformanceTests/Layout/flexbox-column-nowrap.html:31
>> +PerfTestRunner.run(runTest, 180, 20, function() {
> 
> It's fragile to rely on hard-coded values like this.

I'm sorry, I don't understand.  What value should not be hard coded?
Comment 5 Ryosuke Niwa 2012-05-09 14:49:45 PDT
You need done function if you're adding something to the document. run-perf-test assumes any line it doesn't understand as an error message.

What I meant about the hard-coded number is that we should use runs/s as ojan suggested.
Comment 6 Tony Chang 2012-05-09 14:52:57 PDT
(In reply to comment #5)
> What I meant about the hard-coded number is that we should use runs/s as ojan suggested.

Yeah, I didn't want to just copy/paste the code from Dromaeo/resources/dromaeorunner.js.  Ideally, this would be something that resources/runner.js supports natively.  I'll file a bug about this.
Comment 7 WebKit Review Bot 2012-05-09 15:44:37 PDT
Comment on attachment 141012 [details]
Patch

Clearing flags on attachment: 141012

Committed r116564: <http://trac.webkit.org/changeset/116564>
Comment 8 WebKit Review Bot 2012-05-09 15:44:41 PDT
All reviewed patches have been landed.  Closing bug.