Bug 86017 - add some basic perf-o-matic tests for flexbox
Summary: add some basic perf-o-matic tests for flexbox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-09 14:21 PDT by Tony Chang
Modified: 2012-05-09 15:44 PDT (History)
3 users (show)

See Also:

Attachments
Patch (7.00 KB, patch)
2012-05-09 14:23 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

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