1272269 - 20% performance regression since Aurora48, decrypting with pbkdf2 algorithm.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Alice0775 White]

[Tracking Requested - why for this release]: 20% performance regression

+++ This bug was initially created as a clone of Bug #1271947 +++

Build Identifier:
https://hg.mozilla.org/mozilla-central/rev/674a552743785c28c75866969aad513bd8eaf6ae
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0 ID:20160511030221

When I test Bug 1271947, I found a performance regression.


Steps to reproduce:
1. Open attachment 8751602 [details]
2. Click [Test] several times


Actual results:
20% performance regression.

approx. 1000 : Beta47.0b4
approx. 1200 : Nightly49.0a1, Aurora48.0a2



Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=beca0a8904718c01dfe57a758ce2ceccc028dc2b&tochange=7c1f8d3d4f69add3995d27d4c70c92d286aa54b9

Comment 1

[Hannes Verschore [:h4writer]]

Taking

Comment 2

[Hannes Verschore [:h4writer]]

Attachment: Patch

During the refactor I forgot to notice that the NewArray/NewObject vm variants also use the template object to improve performance. I did remove that. This wasn't an issue, since in all those cases we should now be using the shared stubs variant. Except for the "new Array()" call, which cannot use a shared stub. Adding the template object to the all vm calls again (even if this should only be visible on newArrayTryVM called from MCallOptimize.cpp)

Comment 3

[Hannes Verschore [:h4writer]]

Comment on attachment 8751747 [details] [diff] [review]
Patch

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

::: js/src/jit/IonBuilder.cpp
@@ +7278,5 @@
>  
>      // Emit a VM call.
>  
>      gc::InitialHeap heap = gc::DefaultHeap;
> +    MConstant* templateConst = constant(NullValue());

make this again:
MConstant* templateConst = MConstant::New(alloc(), NullValue());

constant already does the "current->add"

@@ +7417,3 @@
>  
>      gc::InitialHeap heap = gc::DefaultHeap;
> +    MConstant* templateConst = constant(NullValue());

make this again:
MConstant* templateConst = MConstant::New(alloc(), NullValue());

constant already does the "current->add"

Comment 4

[Hannes Verschore [:h4writer]]

Alice: thanks for taking the time to look into this!

Comment 5

[Marcia Knous [:marcia]]

Tracking for 48/49 based on the performance regression.

Comment 6

[Jason Orendorff [:jorendorff]]

Thank you, Alice!

Comment 7

[Eric Faust [:efaust]]

Comment on attachment 8751747 [details] [diff] [review]
Patch

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

r=me with the MConstant stuff you mention taken care of.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac69d4a75d6c

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ac69d4a75d6c

Comment 10

[Gerry Chang [:gchang]]

Hi Eric, do you want to uplift this for 48 if these patches are not too risky? If these patches are risky, we might need QE's help to verify.

Comment 11

[Hannes Verschore [:h4writer]]

Attachment: Aurora patch

It was my intention to request approval for aurora. Thanks for the reminder!

Approval Request Comment
[Feature/regressing bug #]:
bug 1241088

[User impact if declined]:
Performance could be lower when using new Array() in FF48 only

[Describe test coverage new/current, TreeHerder]:
Mozilla-inbound for already 3-4 days

[Risks and why]: 
Quite low. It reintroduces an optimization path that was removed by bug 1241088. Made the patch as small as possible.

[String/UUID change made/needed]:
/

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8753831 [details] [diff] [review]
Aurora patch

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

The patch fixes the performance issue. Take it in aurora.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dfe3d2e9bdbe