1794439 - Inline `HasNativeDataPropertyPure` cache lookup in Ion

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P1)

Description

[Alex Thayer [:alexical] (she/her)]

This is commonly called with an atom, as far as my instrumentation was able to observe, so we can cheaply look up the hash without having to worry about looking up the atom by a linear string ptr. Somewhat expectedly, the same is not true of calls to GetNativeDataPropertyByValuePure, so I wasn't able to see much of an improvement by inlining the cache lookup there, unless we wanted to modify our StringToAtomCache to be a fixed size and more masm friendly. I don't think that's a priority though.

Anyway, I ran this through try and saw something like a 5% speedup on the React, React-Redux, and Elm subtests in Speedometer 2, so I think this is worth it.

Comment 1

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1794439 - Inline HasNativeDataPropertyPure cache lookup in Ion r?jandem

I saw something like a 5% improvement to the Elm, React, and React-Redux
subtests of speedometer with this, with maybe a 1% overall Speedometer
improvement, although the confidence is lower.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30dbd4267a26
Inline HasNativeDataPropertyPure cache lookup in Ion r=jandem

Comment 3

[Cristian Tuns]

Backed out for causing multiple failures on BacktrackingAllocator.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: Assertion failure: !minimalBundle(bundle), at /builds/worker/checkouts/gecko/js/src/jit/BacktrackingAllocator.cpp:3484

Log 2 : https://treeherder.mozilla.org/logviewer?job_id=393480982&repo=autoland

Comment 4

[Natalia Csoregi [:nataliaCs]]

Also failing: ERROR - REFTEST ERROR | EXCEPTION: out of memory

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98e7f7bfffc4
Inline HasNativeDataPropertyPure cache lookup in Ion r=jandem

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/98e7f7bfffc4

Comment 7

[Alex Thayer [:alexical] (she/her)]

Eyeballing it, looks like this yeilded wins in react-flavored sub-benchmarks of speedometer, and smaller (~1%) wins in Matrix React, Ares6, and Kraken.