Bug 77051 - Layout/floats.html should be runnable by run-perf-tests
Summary: Layout/floats.html should be runnable by run-perf-tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexandru Chiculita
URL:
Keywords:
Depends on:
Blocks: 77037
  Show dependency treegraph
 
Reported: 2012-01-25 14:47 PST by Ryosuke Niwa
Modified: 2012-03-14 18:17 PDT (History)
3 users (show)

See Also:


Attachments
Patch V1 (16.79 KB, patch)
2012-01-27 05:19 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V2 (17.01 KB, patch)
2012-01-30 05:08 PST, Alexandru Chiculita
rniwa: review+
rniwa: commit-queue-
Details | Formatted Diff | Diff
Rebased patch (17.01 KB, patch)
2012-03-14 17:56 PDT, Alexandru Chiculita
rniwa: review+
Details | Formatted Diff | Diff
Rebase #2 (17.28 KB, patch)
2012-03-14 18:01 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-01-25 14:47:40 PST
run-perf-test should be able to run Layout/floats.html

We might want to consider splitting the test into multiple pieces and use runner.js.
Comment 1 Alexandru Chiculita 2012-01-27 05:19:37 PST
Created attachment 124294 [details]
Patch V1
Comment 2 Ryosuke Niwa 2012-01-27 11:49:30 PST
Comment on attachment 124294 [details]
Patch V1

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

> PerformanceTests/Layout/resources/floats.js:7
> +    var nextRandomIndex = 0;

Maybe we should move a function I'm replacing Math.random with in https://bugs.webkit.org/attachment.cgi?id=124266&action=review to runner.js so you can use it here?
Comment 3 Ryosuke Niwa 2012-01-29 21:41:38 PST
I've committed my patch so you should be able to move move Math.random from
http://trac.webkit.org/browser/trunk/PerformanceTests/DOM/resources/dom-perf.js#L120
to
http://trac.webkit.org/browser/trunk/PerformanceTests/resources/runner.js
Comment 4 Alexandru Chiculita 2012-01-30 05:08:37 PST
Created attachment 124527 [details]
Patch V2

Moved Math.random to runner.js. Also, added a reset function to reset the seed after each run.
Comment 5 Ryosuke Niwa 2012-01-30 09:29:42 PST
Comment on attachment 124527 [details]
Patch V2

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

> PerformanceTests/ChangeLog:9
> +        Some tests take longer to run and I've changed the number of iterations, so that each tests finishes under 1s per run.

Could you split this line into two before landing it? It's really long.

> PerformanceTests/ChangeLog:12
> +            Moved Math.random to runner.js.

We normally append the comment on the same line as the test name and : with one space. e.g.
* DOM/resources/dom-perf.js: Moved Math.random to runner.js.

> PerformanceTests/ChangeLog:36
> +            Moved the Math.random to runner.js to be used by all tests.
> +            Added resetRandomSeed to bring the randomizer back to initial seed.
> +            It is useful to get the same results at every run and minimize the
> +            differences between runs.

And when line wraps, we don't normally indent but instead align them to where ( is in (Math.random). (i.e. indented by exactly 8 spaces from the beginning).
Comment 6 Ryosuke Niwa 2012-01-31 12:59:22 PST
I think this patch is out of date after http://trac.webkit.org/changeset/106379.
Comment 7 Alexandru Chiculita 2012-03-06 11:06:46 PST
(In reply to comment #6)
> I think this patch is out of date after http://trac.webkit.org/changeset/106379.

Sorry, I've been out for a while. Will get back to this one ASAP.
Comment 8 Alexandru Chiculita 2012-03-14 17:56:30 PDT
Created attachment 131966 [details]
Rebased patch

Rebased patch, but changed the code to put the methods in PerfTestRunner, so I'm sending it back to review.
Comment 9 Alexandru Chiculita 2012-03-14 18:01:21 PDT
Created attachment 131967 [details]
Rebase #2
Comment 10 Alexandru Chiculita 2012-03-14 18:08:31 PDT
Landed in r110801: http://trac.webkit.org/changeset/110801