Closed Bug 1267565 Opened 6 years ago Closed 6 years ago
.87 - 4% kraken (linux64, windows7-32, windows8-64, windowsxp) regression on push 09aaae546918 (Mon Apr 25 2016)
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
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?
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 <firstname.lastname@example.org> - 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
Attachment #8745265 - Flags: review?(efaustbmo)
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5decad670c7a535ed949b1553ff8c8a1b8555ffe Bug 1267565 - Re-enable JIT inlining for std_Array. r=efaust
Confirmed mostly fixed. Remaining is probably just noise. https://arewefastyet.com/#machine=28&view=single&suite=kraken&start=1461506398&end=1461761526
and alerts are showing the improvement as well: https://treeherder.mozilla.org/perf.html#/alerts?id=1031 thanks for the quick fix!
You need to log in before you can comment on or make changes to this bug.