Closed Bug 1267565 Opened 6 years ago Closed 6 years ago

2.87 - 4% kraken (linux64, windows7-32, windows8-64, windowsxp) regression on push 09aaae546918 (Mon Apr 25 2016)

Categories

(Core :: JavaScript Engine, defect)

49 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox48 --- unaffected
firefox49 --- fixed

People

(Reporter: jmaher, Assigned: arai)

References

Details

(Keywords: perf, regression, Whiteboard: [talos_regression])

Attachments

(1 file)

Talos has detected a Firefox performance regression from push 09aaae546918. As author of one of the patches included in that push, we need your help to address this regression.

This is a list of all known regressions and improvements related to the push:
https://treeherder.mozilla.org/perf.html#/alerts?id=1010

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/Buildbot/Talos/Tests#kraken

Reproducing and debugging the regression:

If you would like to re-run this Talos test on a potential fix, use try with the following syntax:
try: -b o -p win32,linux64,win64 -u none -t dromaeojs[Windows XP,Windows 7,Windows 8],dromaeojs-e10s[Windows XP,Windows 7,Windows 8] --rebuild 5  # add "mozharness: --spsProfile" to generate profile data

(we suggest --rebuild 5 to be more confident in the results)

To run the test locally and do a more in-depth investigation, first set up a local Talos environment:
https://wiki.mozilla.lorg/Buildbot/Talos/Running#Running_locally_-_Source_Code

Then run the following command from the directory where you set up Talos:
talos --develop -e [path]/firefox -a kraken

Making a decision:
As the patch author we need your feedback to help us handle this regression.
*** Please let us know your plans by Friday, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations:
https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Target Milestone: Firefox 49 → ---
:arai, can you help figure out what is going on here and if we can reduce this regression at all?
Flags: needinfo?(arai.unmht)
as a note the last kraken regression was bug 1262967, we keep getting worse on this test.
With retriggering on awfy I see:
https://arewefastyet.com/#machine=28&view=single&suite=kraken&start=1461620018&end=1461632967

http://hg.mozilla.org/integration/mozilla-inbound/log?rev=45939565472b%3A%3A4b16661bbae5%20and%20!45939565472b

This means it potentially is:
Tooru Fujisawa <arai_a@mac.com> - Tue, 26 Apr 2016 08:08:45 +0900 - rev 294817
Bug 1263525 - Add dedicated function for std_Array self-hosted intrinsic. r=efaust
Sorry, forgot to add the code to retrieve templateObject for array_construct to IonBuilder::inlineArray, and the JIT inlining was disabled.

The inlined JIT code was shared by both Array ctor and std_Array, and the implementation was only ArrayConstructor.  Bug 1267565 separated it into ArrayConstructor and array_construct, and forgot to update IonBuilder::inlineArray.  (I only updated GetTemplateObjectForNative...)

Locally, confirmed the regression in stanford-crypto-pbkdf2 from 98.2 to 114.1, and the fix with this patch.
Assignee: nobody → arai.unmht
Flags: needinfo?(arai.unmht)
Attachment #8745265 - Flags: review?(efaustbmo)
So, this is from bug 1263525.
No longer blocks: 1267364
thanks for the quick fix and narrowing it down!
Comment on attachment 8745265 [details] [diff] [review]
Re-enable JIT inlining for std_Array.

Review of attachment 8745265 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/MCallOptimize.cpp
@@ +437,5 @@
>  
>      JSObject* templateObject = inspector->getTemplateObjectForNative(pc, ArrayConstructor);
> +    // This is shared by ArrayConstructor and array_construct (std_Array).
> +    if (!templateObject)
> +        templateObject = inspector->getTemplateObjectForNative(pc, array_construct);

please add a blank line below this hunk. This |if (!templateObject)| is associated with the decl, not the failure state.
Attachment #8745265 - Flags: review?(efaustbmo) → review+
and alerts are showing the improvement as well:
https://treeherder.mozilla.org/perf.html#/alerts?id=1031

thanks for the quick fix!
https://hg.mozilla.org/mozilla-central/rev/5decad670c7a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.