1591051 - 58.43 - 92.77% perf_reftest_singletons id-getter-3.html (linux64-shippable, linux64-shippable-qr, macosx1014-64-shippable, windows10-64-shippable, windows10-64-shippable-qr) regression on push 02f8ab4ff12457cba2ed3744e45e62867847ad80 (Wed October 23 2019)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Marian Raiciof [:marauder]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=02f8ab4ff12457cba2ed3744e45e62867847ad80

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

93% perf_reftest_singletons id-getter-3.html windows10-64-shippable opt e10s stylo 475.99 -> 917.57
85% perf_reftest_singletons id-getter-3.html macosx1014-64-shippable opt e10s stylo 523.84 -> 967.71
85% perf_reftest_singletons id-getter-3.html windows10-64-shippable-qr opt e10s stylo 476.55 -> 879.56
83% perf_reftest_singletons id-getter-3.html windows10-64-shippable-qr opt e10s stylo 479.96 -> 879.26
60% perf_reftest_singletons id-getter-3.html linux64-shippable opt e10s stylo 439.92 -> 703.70
58% perf_reftest_singletons id-getter-3.html linux64-shippable-qr opt e10s stylo 462.25 -> 732.37

You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=23547

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/TestEngineering/Performance/Talos

For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/TestEngineering/Performance/Talos/Running

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/TestEngineering/Performance/Talos/RegressionBugsHandling

Comment 1

[Jan de Mooij [:jandem]]

Hah, that test has:

  // The shortest string we can't fit in a single-byte inline string (16 chars).
  el.id = "aaaaaaaaaaaaaaaa";

The thin-inline-string limit was bumped from 15 to 16 so this test needs to be fixed (probably by adding an extra 'a').

Issue with the test, not a real regression, I'll fix shortly.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1591051 - Fix some perf tests to account for new thin-inline JS string limit. r?bzbarsky!

Bug 1330776 removed the null-terminator, bumping the limit from 15 to 16.

Comment 3

[Marian Raiciof [:marauder]]

Thank you Jan!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45e0bd294382
Fix some perf tests to account for new thin-inline JS string limit. r=bzbarsky

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/45e0bd294382

Comment 6

[Marian Raiciof [:marauder]]

All looking good, thank you!

== Change summary for alert #23553 (as of Thu, 24 Oct 2019 22:38:53 GMT) ==

Improvements:

47% perf_reftest_singletons id-getter-3.html macosx1014-64-shippable opt e10s stylo 1,015.63 -> 539.35
45% perf_reftest_singletons id-getter-3.html windows10-64-shippable-qr opt e10s stylo 866.67 -> 478.18
44% perf_reftest_singletons id-getter-3.html windows10-64-shippable opt e10s stylo 864.95 -> 485.00
37% perf_reftest_singletons id-getter-3.html linux64-shippable opt e10s stylo 705.50 -> 443.96
36% perf_reftest_singletons id-getter-3.html linux64-shippable-qr opt e10s stylo 730.02 -> 465.75
10% perf_reftest_singletons id-getter-2.html linux64-shippable-qr opt e10s stylo 808.66 -> 730.16

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

Comment 7

[Pascal Chevrel:pascalc (PTO until April 26)]

Marian, are we interested in uplifting the patch for the tests to our beta branch?

Comment 8

[Marian Raiciof [:marauder]]

Hi Pascal, i asked :davehunt about this and i am waiting for his response.

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Pascal Chevrel:pascalc from comment #7)

Marian, are we interested in uplifting the patch for the tests to our beta branch?

No, this doesn't affect 71. I'm not sure why it was marked as being affected.

Comment 10

[Boris Zbarsky [:bzbarsky]]

In particular, uplifting this change to a branch that does not have the fix for bug 1330776 would cause the tests to not longer test the right things, because now the test that's trying to test "just barely short enough to be inline" would no longer be inline.