Bug 86017 – add some basic perf-o-matic tests for flexbox

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Chang]

add some basic perf-o-matic tests for flexbox

[Tony Chang]

Created attachment 141012 [details]
Patch

[Tony Chang]

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.

[Ryosuke Niwa]

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.

[Tony Chang]

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?

[Ryosuke Niwa]

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.

[Tony Chang]

(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.

[WebKit Review Bot]

Comment on attachment 141012 [details]
Patch

Clearing flags on attachment: 141012

Committed r116564: <http://trac.webkit.org/changeset/116564>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.