1380281 - Don't abort with "NYI: arguments & setarg."

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P3)

Description

[André Bargull [:anba]]

Test case:
---
class Matrix {
    constructor(nRows, nColumns) {
        if (arguments.length === 1 && typeof nRows === 'number') {
            return new Array(nRows);
        }
        if (Number.isInteger(nRows) && nRows > 0) {
            // [Deleted]
        } else if (Array.isArray(nRows)) {
            const matrix = nRows;
            nRows = matrix.length;
            // [Deleted]
        }
    }
}

for (var i = 0; i < 1000000; ++i)
    new Matrix(2, 2);
---



Ion compilations is aborted with:
---
[IonAbort] NYI: arguments & setarg.
[IonAbort] aborted @ /tmp/q.js:10
[IonAbort] Disabling Ion compilation of script /tmp/q.js:2
---


This issue shows up in ARES-6/ml: 
https://github.com/WebKit/webkit/blob/c784f451cfc259f2fac307000f846b0146bb780e/PerformanceTests/ARES-6/ml/index.js#L4556-L4573

Comment 1

[Tom S. (please needinfo tschuster)]

Seems like we first tried this in bug 921120, but it was disabled again in bug 957475 for crashes and sec-critical. Maybe Kannan wants to try this again?

Comment 2

[André Bargull [:anba]]

Looks like a ~40% improvement on ARES-6/ml when this gets fixed. Sigh.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1380281 - Part 1: Support JSOp::SetArg with unmapped arguments object. r=jandem!

When the arguments object is unmapped, JSOp::SetArg can directly write to the
arguments slot, because we don't need to synchronise it with the arguments
object.

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1380281 - Part 2: Decide within the arguments analysis if unmapped arguments require an arguments object. r=jandem!

This change just moves the point where we decide if parameter modification
combined with an unmapped arguments object requires to eagerly create an
arguments object. It shouldn't result in any change of behaviour.

Depends on D64574

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1380281 - Part 3: Only create an unmapped arguments object when the arguments contents are observed. r=jandem!

When the script contains JSOp::SetArg and uses unmapped arguments objects, we
can still omit creating the arguments object as long as the arguments contents
are never observed.

Depends on D64575

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddd4454e71c0
Part 1: Support JSOp::SetArg with unmapped arguments object. r=jandem
https://hg.mozilla.org/integration/autoland/rev/6a01bbfc11f7
Part 2: Decide within the arguments analysis if unmapped arguments require an arguments object. r=jandem
https://hg.mozilla.org/integration/autoland/rev/098eb676052f
Part 3: Only create an unmapped arguments object when the arguments contents are observed. r=jandem

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/ddd4454e71c0
https://hg.mozilla.org/mozilla-central/rev/6a01bbfc11f7
https://hg.mozilla.org/mozilla-central/rev/098eb676052f

Comment 8

[Marian Raiciof [:marauder]]

== Change summary for alert #25248 (as of Sun, 01 Mar 2020 21:04:12 GMT) ==

Improvements:

13% raptor-ares6-firefox macosx1014-64-shippable opt 69.99 -> 61.13
10% raptor-ares6-firefox linux64-shippable-qr opt 55.15 -> 49.58
9% raptor-ares6-firefox windows7-32-shippable opt 57.60 -> 52.27
9% raptor-ares6-firefox windows10-64-shippable opt 57.98 -> 52.82
9% raptor-ares6-firefox linux64-shippable opt 54.17 -> 49.48
8% raptor-ares6-firefox windows10-64-shippable-qr opt 57.55 -> 52.78
8% raptor-ares6-firefox windows7-32-shippable opt 57.49 -> 52.76

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

Comment 9

[Florin Strugariu [:Bebe]]

== Change summary for alert #25251 (as of Mon, 02 Mar 2020 10:52:31 GMT) ==

Improvements:

13% raptor-ares6-firefox windows10-64 opt 64.94 -> 56.25
12% raptor-ares6-firefox windows10-64-qr opt 64.15 -> 56.17
11% raptor-ares6-firefox linux64 opt 57.97 -> 51.74
10% raptor-ares6-firefox linux64-qr opt 57.68 -> 51.81
10% raptor-ares6-firefox windows10-64-shippable-qr opt 57.46 -> 51.67
10% raptor-ares6-firefox macosx1014-64-shippable opt 69.66 -> 62.70
10% raptor-ares6-firefox linux64-shippable opt 54.75 -> 49.28
10% raptor-ares6-firefox windows7-32-shippable opt 57.81 -> 52.17
10% raptor-ares6-firefox windows7-32 opt 63.29 -> 57.20
10% raptor-ares6-firefox linux64-shippable-qr opt 54.39 -> 49.19
9% raptor-ares6-firefox linux64-shippable opt 54.54 -> 49.74

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