Add graphics microbenchmarking

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
Based on my conversation with wlach, we are now in a state where we should be able to trivially add in-tree microbenchmarking tests, using the GTest framework, and report this data to perfherder.

There's a lot of special consideration that come with microbenchmarking. Most importantly is a sheriffing policy. Since microbenchmarking are very sensitive to small change that are not user visible regressions they should have a more relaxed back out policy. An appropriate sheriff policy will be setup as these test go to production.
Created attachment 8730335 [details] [review]
[treeherder] wlach:1256408 > mozilla:master
Comment on attachment 8730335 [details] [review]
[treeherder] wlach:1256408 > mozilla:master

This will need to be enabled on treeherder stage before we can start testing this.

:BenWa: Does platform_microbench sound ok as a grouping name?
Attachment #8730335 - Flags: review?(jmaher)
Attachment #8730335 - Flags: feedback?(bgirard)
Comment on attachment 8730335 [details] [review]
[treeherder] wlach:1256408 > mozilla:master

just make sure we deploy this :)
Attachment #8730335 - Flags: review?(jmaher) → review+
(Assignee)

Comment 4

3 years ago
(In reply to William Lachance (:wlach) from comment #2)
> Comment on attachment 8730335 [details] [review]
> [treeherder] wlach:1256408 > mozilla:master
> 
> This will need to be enabled on treeherder stage before we can start testing
> this.
> 
> :BenWa: Does platform_microbench sound ok as a grouping name?

Yes, but it might be odd if non platform stuff starts using it.
(Assignee)

Comment 5

3 years ago
Perfherder schema:

08:16:34     INFO - PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"subtests": [{"name": "libxul.so", "value": 93013840}], "name": "installer size", "value": 60343406}]}

http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-beta-linux64-debug/1457962948/mozilla-beta-linux64-debug-bm74-build1-build5.txt.gz

https://github.com/mozilla/treeherder/blob/master/schemas/performance-artifact.json
(Assignee)

Updated

3 years ago
Assignee: nobody → bgirard
(Assignee)

Comment 6

3 years ago
Created attachment 8730378 [details]
Smple output

How does this look?
Attachment #8730378 - Flags: feedback?(wlachance)
(Assignee)

Comment 8

3 years ago
Created attachment 8730386 [details]
MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

Review commit: https://reviewboard.mozilla.org/r/39831/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39831/
(Assignee)

Updated

3 years ago
Flags: needinfo?(bgirard)
Attachment #8730378 - Flags: feedback?(wlachance)
(Assignee)

Updated

3 years ago
Flags: needinfo?(bgirard)
Attachment #8730335 - Flags: feedback?(bgirard)
(Assignee)

Comment 10

3 years ago
Looks like the logs are hanging on parse? Or maybe just a backlog?
Flags: needinfo?(wlachance)
(In reply to Benoit Girard (:BenWa) from comment #10)
> Looks like the logs are hanging on parse? Or maybe just a backlog?

It took me a while to figure out what's wrong. It's actually pretty silly. :) There's a stray ':' in your json under the "name" property for the suite.

{"framework": {"name": "platform_microbench"}, "suites": [{"name:": "GfxBench", ...
                                                            ^^^^^^

Fix that and everything should be good.
Flags: needinfo?(wlachance)
(Assignee)

Comment 12

3 years ago
Comment on attachment 8730386 [details]
MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39831/diff/1-2/
(Assignee)

Updated

3 years ago
Attachment #8730386 - Attachment description: MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r? → MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r?milan
Attachment #8730386 - Flags: review?(milan)
(Assignee)

Comment 13

3 years ago
Comment on attachment 8730386 [details]
MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39831/diff/2-3/
(Assignee)

Comment 14

3 years ago
Results look good. Doing a few re-triggers to see how stable the results are:
https://treeherder.allizom.org/#/jobs?repo=try&revision=11c4b8adf0dd&selectedJob=18676584
(Assignee)

Comment 15

3 years ago
Comment on attachment 8730386 [details]
MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

Since Milan is away I'm asking mstange. No need to wait for a small patch.
Flags: needinfo?(bgirard)
Attachment #8730386 - Flags: review?(milan) → review?(mstange)
Comment on attachment 8730386 [details]
MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

https://reviewboard.mozilla.org/r/39831/#review36997

The specific benchmarks you're adding don't look all that useful to me, but at least it gives us something to start playing with.

::: gfx/tests/gtest/TestCompositor.cpp:232
(Diff revision 3)
>  {
>    auto layerManagers = GetLayerManagers(GetPlatformBackends());
>  }
>  
> -TEST(Gfx, CompositorSimpleTree)
> -{
> +static void CompositorSimpleTree() {
> +  const int size = 30;

I'd call this variable "benchmarkRepeatCount" or something that conveys that this is not a geometric size.
Attachment #8730386 - Flags: review?(mstange) → review+
(Assignee)

Comment 17

3 years ago
I've added some wiki information:
https://wiki.mozilla.org/Buildbot/Talos/Tests#Microbench
https://wiki.mozilla.org/Buildbot/Talos/Sheriffing#Microbench_Policy

(I'll probably split these off shortly)

(In reply to Markus Stange [:mstange] from comment #16)
> Comment on attachment 8730386 [details]
> MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r?milan
> 
> https://reviewboard.mozilla.org/r/39831/#review36997
> 
> The specific benchmarks you're adding don't look all that useful to me, but
> at least it gives us something to start playing with.

They're not great, but they're not awful either. Having good microbenchmark test is hard(tm), but we can mitigate this by having many of them. We have occasionally modified the Region code. The compositor test is a bit better. It will construct a lot of layers and composite them. A regression in that area should be picked up by this test.

But it will be more useful to add them once we have a particular performance regressions that we are interested in watching.
(Assignee)

Comment 18

3 years ago
Comment on attachment 8730386 [details]
MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39831/diff/3-4/
Attachment #8730386 - Attachment description: MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r?milan → MozReview Request: Bug 1256408 - Add graphics microbenchmarking. r=mstange

Comment 20

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/396639988f51
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.