1596172 - Add perf-reftest-singletons to PGO training

Status: RESOLVED FIXED

(Firefox Build System :: Toolchains, enhancement)

Description

[(Away)]

Recent changes in bug 1592981 caused a regression in the perf-reftest-singletons, mostly because nsStringBuffer::Release() stopped getting inlined. Fixing that is not quite as simple as tossing a MOZ_ALWAYS_INLINE on there: since Release() is a class method, it has different semantics about inline, so you get linker errors with the naive approach.

I tried reworking the function in various ways, but the compiler still wouldn't give me the code I wanted. Knowing that profile data can override bug 1592981's settings, I propose adding the test suite to PGO.

Also I'm hoping that this will increase the stability of this test suite as well. The singletons have been particularly fussy about toolchains updates in the past, and I'd prefer not to keep chasing after fluctuations in them.

Comment 1

[(Away)]

Attachment: Add perf-reftest-singletons to PGO training

This ought to overcome a regression from 1592981 as well as make this test suite less noisy about toolchain updates in the future.

While here, tidy up the PGO list: sort alphabetically, stop using a hardcoded test index for the Speedometer time extension, and stop using js-input/ for the virtual mappings since js-input/ is an actual directory in our repo.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ee409403100
Add perf-reftest-singletons to PGO training r=firefox-build-system-reviewers,mshal

Comment 3

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/0ee409403100

Comment 4

[Florin Strugariu [:Bebe]]

== Change summary for alert #23969 (as of Fri, 15 Nov 2019 10:15:28 GMT) ==

Improvements:

24% perf_reftest_singletons external-string-pass.html linux64-shippable-qr opt e10s stylo 994.16 -> 757.21
24% perf_reftest_singletons external-string-pass.html linux64-shippable-qr opt e10s stylo 990.38 -> 755.36
24% perf_reftest_singletons external-string-pass.html linux64-shippable opt e10s stylo 958.72 -> 732.86
8% perf_reftest_singletons tiny-traversal-singleton.html linux64-shippable opt e10s stylo 836.80 -> 772.42
7% perf_reftest_singletons id-getter-6.html linux64-shippable opt e10s stylo 454.92 -> 421.51
7% perf_reftest_singletons id-getter-4.html linux64-shippable opt e10s stylo 453.94 -> 421.02
7% perf_reftest_singletons id-getter-5.html linux64-shippable opt e10s stylo 454.01 -> 421.21
7% perf_reftest_singletons id-getter-4.html linux64-shippable-qr opt e10s stylo 477.33 -> 444.93
7% perf_reftest_singletons id-getter-3.html linux64-shippable-qr opt e10s stylo 476.36 -> 444.14
7% perf_reftest_singletons id-getter-3.html linux64-shippable opt e10s stylo 453.65 -> 423.45
7% perf_reftest_singletons id-getter-6.html linux64-shippable-qr opt e10s stylo 475.80 -> 444.59
7% perf_reftest_singletons scrollbar-styles-1.html linux64-shippable opt e10s stylo 494.23 -> 462.10
6% perf_reftest_singletons id-getter-5.html linux64-shippable-qr opt e10s stylo 476.60 -> 446.65
6% perf_reftest_singletons id-getter-7.html linux64-shippable opt e10s stylo 452.33 -> 424.54
6% perf_reftest_singletons scrollbar-styles-1.html linux64-shippable-qr opt e10s stylo 515.41 -> 484.65
6% perf_reftest_singletons id-getter-7.html linux64-shippable-qr opt e10s stylo 474.48 -> 446.51

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=23969

Comment 5

[Boris Zbarsky [:bzbarsky]]

David, it looks like this basically copied the singletons manifest, but it's already out of sync since more singletons tests got added. The whole point of reftest-singletons is that people should add tests easily, and the expectation is that we should keep doing that.

Is there some technical way to keep the two lists in sync? If not, should we have warning comments around the two lists about keeping them in sync?

Comment 6

[(Away)]

I wasn't aware that these tests were actively changing. I had been getting alerts on the same set of subtests for ages so I assumed they were developed long ago and forgotten. My fault.

I think ideally both raptor and PGO training would load up the same single harness that knows how to iterate over the right set of things. IIRC the current harness assumes the existence of a bunch of raptor/talos machinery so we'd have to factor that out in order to be reused by the PGO run.

Warning comments seem less preferable but might be OK in a pinch.