1553744 - 0.3 - 0.39% Base Content JS (linux64-shippable, linux64-shippable-qr, macosx1010-64-shippable, windows10-64-shippable, windows10-64-shippable-qr, windows7-32-shippable) regression on push a8f5dec91d72d4032ced008f847ead37f350193f

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Ionuț Goldan [:igoldan]]

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=350f7249407d5156f831562b38807111790e8e74&tochange=9953e8853243fe62ad2d74d30df1231cf5c5898f

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

Regressions:

0.4% Base Content JS windows7-32-shippable opt 3,233,253.33 -> 3,245,760.00
0.3% Base Content JS macosx1010-64-shippable opt 4,084,846.67 -> 4,097,484.00
0.3% Base Content JS linux64-shippable opt 4,083,844.00 -> 4,096,522.67
0.3% Base Content JS windows10-64-shippable-qr opt 4,140,018.67 -> 4,152,893.33
0.3% Base Content JS windows10-64-shippable opt 4,140,072.00 -> 4,153,030.67
0.3% Base Content JS linux64-shippable-qr opt 4,084,141.33 -> 4,096,592.00

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

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 jobs in a pushlog format.

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

Comment 1

[Ionuț Goldan [:igoldan]]

:khyperia if one of the Regressed by bugs is not that related, you can remove it from that list, so we're precise.

Comment 2

[Ashley Hauck [:khyperia]]

Bug 1552229 - Emit field keys in correct order. is likely irrelevant.

Bug 1547467 - Introduce another scope for .initializers, and remove .localInitializers. added an unconditional scope around every explicit constructor, to support the implementation of fields - we couldn't make it conditional to only classes with fields, due to the constrained architectural requirements of the parser, without doing some pretty convoluted and difficult-to-maintain hacks.

However, as this did get flagged as a significant memory regression, I have a patch to do a convoluted hack to go back to the memory usage before this patch, by removing that scope in classes without fields.

Comment 3

[Ashley Hauck [:khyperia]]

Attachment: Bug 1553744 - Delete .initalizers scope for constructors in classes without fields.

Comment 4

[Steven DeTar [:sdetar]]

Ashley, I setting the priority of this bug to P1.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ee40acc8b60
Delete .initalizers scope for constructors in classes without fields. r=jorendorff

Comment 6

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/4ee40acc8b60

Comment 7

[Florin Strugariu [:Bebe]]

== Change summary for alert #21192 (as of Fri, 31 May 2019 10:55:10 GMT) ==

Improvements:

0% Base Content JS windows7-32-shippable opt 3,148,234.67 -> 3,135,714.67
0% Base Content JS macosx1010-64-shippable opt 3,988,421.33 -> 3,974,598.67
0% Base Content JS linux64-shippable opt 3,987,282.67 -> 3,973,520.00
0% Base Content JS linux64-shippable-qr opt 3,987,336.00 -> 3,973,680.00
0% Base Content JS linux64-shippable opt 3,986,935.33 -> 3,973,888.00
0% Base Content JS windows10-64-shippable-qr opt 4,043,532.00 -> 4,030,672.00
0% Base Content JS windows10-64-shippable opt 4,043,676.00 -> 4,030,577.33

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

Comment 8

[Florin Strugariu [:Bebe]]

Congrats for the fix