Closed Bug 1422010 Opened 2 years ago Closed 2 years ago

Run StyleBench on Talos

Categories

(Testing :: Talos, enhancement)

Version 3
enhancement
Not set

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: emilio, Assigned: jmaher)

References

(Blocks 1 open bug)

Details

(Whiteboard: [PI:January][stylo:p2])

Attachments

(2 files, 1 obsolete file)

WebKit just added a style system performance benchmark using the Speedometer infrastructure:

  https://trac.webkit.org/changeset/225324/webkit

We're faster than Blink / WebKit on my machine at least, but it'd be nice to run this on automation so we don't regress it, and to track improvements.
Joel, how hard would this be to integrate?
Flags: needinfo?(jmaher)
this looks to measure dom mutations, we have dromaeo-dom and kraken running, are those the same thing?  I am fine updating or replacing benchmarks, my goal is to avoid duplication.

It is easy to add a test to Talos, and somewhat easy for AWFY.  Do we need it on AWFY or only Talos?  AWFY has very limited hardware available, so the more we add there the less frequently we can run.  AWFY is good for multi browser compare and publishing numbers- Talos is good for finding regressions and tracking improvements.

Can you confirm the lack of duplication or instruct me on what we could remove when adding this?
Flags: needinfo?(jmaher)
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #2)
> this looks to measure dom mutations, we have dromaeo-dom and kraken running,
> are those the same thing?  I am fine updating or replacing benchmarks, my
> goal is to avoid duplication.

They're definitely not the same. This benchmark tests incremental style updates, which is something that dromaeo-dom and kraken don't do (they just measure the raw performance of the dom mutation, not the actual performance of the rendering update).

Similarly, we have dromaeo-css to test selector-matching performance, but that basically tests querySelector and not the style system as a whole.

So I don't think there's any duplication, and indeed the lack of tests for this is what made me file bug 1372533.

> It is easy to add a test to Talos, and somewhat easy for AWFY.  Do we need
> it on AWFY or only Talos?  AWFY has very limited hardware available, so the
> more we add there the less frequently we can run.  AWFY is good for multi
> browser compare and publishing numbers- Talos is good for finding
> regressions and tracking improvements.

I think we do want it on Talos. Less sure about AWFY. Just Talos would be ok for me, but if anyone else thinks differently...
Blocks: 1372533
ok, great- this is doable.  I have a session in Austin for hacking on talos as there is interest in debugging, adding/editing tests, etc.  Would you like to participate in that and get this running there?  You can also see prior art in bug 1395247 for how we added speedometer to talos :)

I think step #1 is getting the sources checked in-tree, then we can start experimenting.
Whiteboard: [PI:December]
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #4)
> ok, great- this is doable.  I have a session in Austin for hacking on talos
> as there is interest in debugging, adding/editing tests, etc.  Would you
> like to participate in that and get this running there?  You can also see
> prior art in bug 1395247 for how we added speedometer to talos :)

I'd be glad to :)

> I think step #1 is getting the sources checked in-tree, then we can start
> experimenting.

Sounds good! Will file. We already have Speedometer so it shouldn't be hard.
Depends on: 1422024
Joel, can someone on your team help add StyleBench to Talos in January? I don't know when Emilio will have time because he is pretty busy fixing some Stylo performance bugs.
Flags: needinfo?(jmaher)
We can do this in early January
Flags: needinfo?(jmaher)
Whiteboard: [PI:December] → [PI:January]
Summary: Run StyleBench on Talos / AWFY → Run StyleBench on Talos and/or AWFY
Whiteboard: [PI:January] → [PI:January][stylo:p2]
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #8943247 - Flags: review?(rwood)
Comment on attachment 8943247 [details] [diff] [review]
copy over all the webkit benchmarks into the talos tests folder

Review of attachment 8943247 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mozharness/mozharness/mozilla/testing/talos.py
@@ +439,1 @@
>          if not os.path.exists(dest):

The first time I ran this it failed to copy over the webkit tests because I already had a local testing/talos/talos/tests/webkit/PerformanceTests with speedometer in it. So the first time this patch is applied, if speedometer was run locally previously, then stylebench fails with URL not found.

Maybe have to check for each sub-folder and if any of those are missing then copy it over.

Also if any of the webkit tests are updated they won't automatically be copied over when running locally... do you think that may be an issue? Maybe not because they probably won't be updated much if at all...
Attachment #8943247 - Flags: review?(rwood) → review-
Comment on attachment 8943250 [details] [diff] [review]
run stylebench on talos

Review of attachment 8943250 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with question resolved re: adding to speedometer

::: testing/talos/talos.json
@@ +83,5 @@
>              "tests": ["perf_reftest_singletons"],
>              "talos_options": ["--disable-stylo"]
>          },
>          "speedometer-e10s": {
> +            "tests": ["speedometer", "stylebench"]

Did you mean to add it to speedometer or was this just temporary so it can be run on try?
Attachment #8943250 - Flags: review?(rwood) → review+
updated to remove the webkit folder all the time when using mach before copying over.
Attachment #8943247 - Attachment is obsolete: true
Attachment #8943310 - Flags: review?(rwood)
Comment on attachment 8943310 [details] [diff] [review]
copy over all the webkit benchmarks into the talos tests folder

Review of attachment 8943310 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, tried locally, works great
Attachment #8943310 - Flags: review?(rwood) → review+
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1e7990c2c49
copy over all webkit benchmarks to talos during build. r=rwood
https://hg.mozilla.org/integration/mozilla-inbound/rev/3789470ffae0
Run StyleBench on Talos. r=rwood
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #9)
> here is a compare view to see the noise...a bit higher than I would like,
> but not too bad:
> https://treeherder.mozilla.org/perf.html#/
> compare?originalProject=try&originalRevision=1500fda300c557b74d7f79cc70e2c547
> 924899d1&newProject=try&newRevision=1500fda300c557b74d7f79cc70e2c547924899d1&
> framework=1

Thanks for adding these tests, Joel!

Hiro has some StyleBench patches in bug 1425058 that should make the tests less noisy.

@ Hiro, should we land your StyleBench patches in mozilla-central and/or submit them to upstream WebKit? Here is the WebKit bug where they landed StyleBench:

https://bugs.webkit.org/show_bug.cgi?id=180140
Flags: needinfo?(hikezoe)
Does the 'noise' mean variations in the several runs?  Anyway I will do try runs whether my patch reduces variations. Keep ni.
Oh, the commit has not landed on m-c yet, I did push a try without running the test. :)
https://hg.mozilla.org/mozilla-central/rev/e1e7990c2c49
https://hg.mozilla.org/mozilla-central/rev/3789470ffae0
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
:emilio- can you add a section to the test wiki:
https://wiki.mozilla.org/Performance_sheriffing/Talos/Tests

We need to know:
* what the test is measuring
* contact/owner
* any calculations it is doing
Flags: needinfo?(emilio)
I put some details on that page, thanks!
Flags: needinfo?(emilio)
Now that StyleBench is running smoothly on Talos, it would be good to also run it on AWFY so we can track changes in other browsers' performance. I'll file a follow up bug.
Summary: Run StyleBench on Talos and/or AWFY → Run StyleBench on Talos
Duplicate of this bug: 1372533
You need to log in before you can comment on or make changes to this bug.