Closed Bug 1380281 Opened 7 years ago Closed 4 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox75 --- fixed

People

(Reporter: anba, Assigned: anba)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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
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?
Priority: -- → P3
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Depends on: 1478350

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

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.

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

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

Pushed by cbrindusan@mozilla.com:
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

== 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

== 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

Regressions: 1620189
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: