1378139 - Add a stylo-only talos test suite for perf-reftest-singletons

Status: RESOLVED FIXED

(Testing :: Talos, enhancement)

Description

[Robert Wood [:rwood]]

In Bug 1363104 we changed the talos 'perf-reftest' suite and bloom_basic test so that the test measures both bloom_basic and bloom_basic_ref and reports the comparison of those two, instead of reporting separate raw values for each.

Now for stylo only, we want a new talos test suite that will run just bloom_basic on it's own and report only the raw values of that test. This bug is for the talos additions for that. There will be a separate bug filed for the corresponding buildbot/treeherder config changes required.

Comment 1

[Bobby Holley (:bholley)]

(In reply to Robert Wood [:rwood] from comment #0)
> In Bug 1363104 we changed the talos 'perf-reftest' suite and bloom_basic
> test so that the test measures both bloom_basic and bloom_basic_ref and
> reports the comparison of those two, instead of reporting separate raw
> values for each.
> 
> Now for stylo only, we want a new talos test suite that will run just
> bloom_basic on it's own

Ideally, the suite would be extensible enough that we could add a few other things in addition to bloom_basic without much trouble.

Comment 2

[Robert Wood [:rwood]]

In reply to Bobby Holley (:bholley) from https://bugzilla.mozilla.org/show_bug.cgi?id=1363104#c32

Ok thanks, I will use the updated 'bloom_basic.html' from [1]. For the suite name I like 'perf-reftest-singletons' or maybe 'perf-reftest-absolutes'.

[1] https://github.com/heycam/style-perf-tests/tree/master/perf-reftest

Comment 3

[Bobby Holley (:bholley)]

Sounds great, thanks!

Comment 4

[Robert Wood [:rwood]]

Attachment: Bug 1378139 - Add stylo-only talos test suite for perf-reftest singletons;

Review commit: https://reviewboard.mozilla.org/r/154534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154534/

Comment 5

[Ionuț Goldan [:igoldan]]

Comment on attachment 8883596 [details]
Bug 1378139 - Add stylo-only talos test suite for perf-reftest singletons;

https://reviewboard.mozilla.org/r/154534/#review160200

Tested locally with success, with mach talos-tests and in standalone mode.
Code looks good and easy to follow.

Comment 6

[Robert Wood [:rwood]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=738c824a91bf62ca280f9018ca4de5546ba06580

Comment 7

[Robert Wood [:rwood]]

Hey :bholley,

I grabbed your latest bloom_basic.html and util.js from the repo in comment 2. When landing these on try there are a bunch of ES lint errors [1].

Would you mind addressing these in your repo? Otherwise everytime I grab an updated version from your repo I will need to re-fix them. I can sync your repo and fix them if you like, however I'm not sure how to address the 'xxxx not defined' errors.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=738c824a91bf62ca280f9018ca4de5546ba06580&selectedJob=112557492

Thanks!

Comment 8

[Bobby Holley (:bholley)]

(In reply to Robert Wood [:rwood] from comment #7)
> Hey :bholley,
> 
> I grabbed your latest bloom_basic.html and util.js from the repo in comment
> 2. When landing these on try there are a bunch of ES lint errors [1].
> 
> Would you mind addressing these in your repo? Otherwise everytime I grab an
> updated version from your repo I will need to re-fix them. I can sync your
> repo and fix them if you like, however I'm not sure how to address the 'xxxx
> not defined' errors.

Hm, I'm not sure how the linter works, but those 'not defined' things are defined in util.rs...

Fastest is probably for you to figure out what's going on there and then PR any changes you need to the github repo (which is managed by heycam). He's quite responsive on IRC if you need a quick merge.

Comment 9

[Robert Wood [:rwood]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a68aa4cf85ff9831498e54f64fa284d20b663e21

Comment 10

[Robert Wood [:rwood]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0277ba3a331ebed24bae566e3f6297d078b7f43a

Comment 11

[Robert Wood [:rwood]]

Attachment: https://github.com/heycam/style-perf-tests/pull/6

PR in style-perf-tests to fix bloom-basic and util.js eslint errors

Comment 12

[Robert Wood [:rwood]]

Comment on attachment 8883596 [details]
Bug 1378139 - Add stylo-only talos test suite for perf-reftest singletons;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154534/diff/1-2/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7022e032fb01
Add stylo-only talos test suite for perf-reftest singletons; r=igoldan

Comment 14

[Robert Wood [:rwood]]

Hey :bholley, the perf-reftest-singletons will always use the same *.html test files as the /perf-reftest (comparison) tests will correct?

I have the /perf-reftest-singleton manifest pointing to /perf-reftest/bloom_basic.html which makes sense to me, to not have two copies of the same test. I uploaded the latest bloom_basic.html and utils.js to /perf-reftest-singletons though - which I realize is no point. So I'll add another patch to update bloom_basic in perf-reftest to the latest instead if you agree.

Comment 15

[Cameron McCormack (:heycam)]

Comment on attachment 8884382 [details] [review]
https://github.com/heycam/style-perf-tests/pull/6

Merged.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7022e032fb01

Comment 17

[Bobby Holley (:bholley)]

(In reply to Robert Wood [:rwood] from comment #14)
> Hey :bholley, the perf-reftest-singletons will always use the same *.html
> test files as the /perf-reftest (comparison) tests will correct?
> 
> I have the /perf-reftest-singleton manifest pointing to
> /perf-reftest/bloom_basic.html which makes sense to me, to not have two
> copies of the same test. I uploaded the latest bloom_basic.html and utils.js
> to /perf-reftest-singletons though - which I realize is no point. So I'll
> add another patch to update bloom_basic in perf-reftest to the latest
> instead if you agree.

I think it makes sense to keep the actual tests separate. The fact that bloom-basic currently is useful as both a perf-reftest and as a singleton is a bit of an outlier, and the other two singleton tests that I added don't make sense as perf-reftests. Moreover, it may end up useful to tweak the singleton version of bloom-basic with different parameters (to make it less noisy).

That said, we should share util.js if at all possible.

Comment 18

[Robert Wood [:rwood]]

Ok thanks for the info! Reopening, to fix the manifest to point to the new singleton bloom_basic, not the one in /perf-reftest

Comment 19

[Robert Wood [:rwood]]

Attachment: fixmanifest.patch

Comment 20

[Ionuț Goldan [:igoldan]]

Comment on attachment 8884954 [details] [diff] [review]
fixmanifest.patch

Again, the actual test ran OK. I also ran | mach lint --linter eslint --quiet |, which didn't report any issues.

Comment 21

[Robert Wood [:rwood]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/16d2d4153a69d936bda83e9f37e204002d307983
Bug 1378139 - Fix talos perf-reftest-singletons manifest; r=igoldan

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/16d2d4153a69