2000328 - Add macroassembler support for WeakMap lookup

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Iain Ireland [:iain]]

One of the biggest remaining opportunities for speeding up WeakMap lookup is to inline it into jitcode. This is somewhat tricky because WeakMaps are implemented using MFBT hashtables, which don't currently have any JIT support, but that's fixable with enough elbow grease.

I have a mostly working prototype, which reduces the time per lookup in the benchmark attached to bug 1821107 from 20ns/lookup to 13ns/lookup, bringing us to parity with V8.

This should also open up the opportunity to inline hashtable lookup in other cases in the future.

Comment 1

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Implement prepareHashMFBT r=jandem

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Remove bitfield in mozilla::detail::HashTable r=jandem

We need to load the hash shift from JIT code, which requires a known offset. Explicit shifting is more reliable than bitfields.

Testing on godbolt (https://godbolt.org/z/srvaqfoPb), it looks like this will generally produce very slightly nicer code than before. Previously, the hashShift bitfield was being stored at the most-significant end of the word. To load mGen, we therefore had to mask off the hash shift with a large 64-bit constant. Now we can just do a shift, which is the same number of instructions but a smaller encoding.

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Add MacroAssembler implementations of hash1/hash2/applyDoubleHash r=jandem

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Implement MacroAssembler::lookupMFBT r=jandem

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Remove unused argument from emitPreBarrierFastPath r=jandem

Bug 1740745 removed the last use of this argument.

Driveby: remove unnecessary arch-specific code

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Implement MacroAssembler::emitValueReadBarrierFastPath r=jandem!,jonco!

This is used in the next patch.

Comment 7

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Implement WeakMapObject::getObject inline r=jandem

I added the new tests in weakmapset-get-has.js because my first version of this patch was checking for a null pointer when the hash map didn't exist, instead of undefined, and the only testcase that seemed to catch it was a non262 test marked slow.

Comment 8

[Iain Ireland [:iain]]

Attachment: Bug 2000328: Implement WeakMapObject::hasObject inline r=jandem

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d8da973e94d8
https://hg.mozilla.org/integration/autoland/rev/c861214b84e2
Implement prepareHashMFBT r=jandem
https://github.com/mozilla-firefox/firefox/commit/5414c2911e36
https://hg.mozilla.org/integration/autoland/rev/97c6eb9cdb18
Remove bitfield in mozilla::detail::HashTable r=jandem
https://github.com/mozilla-firefox/firefox/commit/102bbcb8815e
https://hg.mozilla.org/integration/autoland/rev/cf5fb6b3a4c5
Add MacroAssembler implementations of hash1/hash2/applyDoubleHash r=jandem
https://github.com/mozilla-firefox/firefox/commit/147f2316380c
https://hg.mozilla.org/integration/autoland/rev/2edd45b64a2b
Implement MacroAssembler::lookupMFBT r=jandem
https://github.com/mozilla-firefox/firefox/commit/696cbdd758e9
https://hg.mozilla.org/integration/autoland/rev/0fa5d09e23da
Remove unused argument from emitPreBarrierFastPath r=jandem
https://github.com/mozilla-firefox/firefox/commit/28f7bc6772ae
https://hg.mozilla.org/integration/autoland/rev/7d4b9516c141
Implement MacroAssembler::emitValueReadBarrierFastPath r=jandem,jonco
https://github.com/mozilla-firefox/firefox/commit/757f7f0b0abc
https://hg.mozilla.org/integration/autoland/rev/89c720e906da
Implement WeakMapObject::getObject inline r=jandem
https://github.com/mozilla-firefox/firefox/commit/f40a0385489b
https://hg.mozilla.org/integration/autoland/rev/7ee250557def
apply code formatting via Lando

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c4efb2b73d8e
https://hg.mozilla.org/integration/autoland/rev/a19a69729816
Revert "Bug 2000328: apply code formatting via Lando" for causing SM Bustages on CodeGenerator.cpp

Comment 11

[Norisz Fay [:noriszfay]]

Backed out for causing SM Bustages on CodeGenerator.cpp

Backout link

Push with failures

Failure log

Comment 12

[Iain Ireland [:iain]]

Oops, turned out I accidentally landed the second-last patch in the stack, which would have been fine except that it contained a syntax error that was fixed in the last patch. Relanding the whole stack now.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8c89daa76c67
https://hg.mozilla.org/integration/autoland/rev/d06c70b12058
Implement prepareHashMFBT r=jandem
https://github.com/mozilla-firefox/firefox/commit/25de8c18648c
https://hg.mozilla.org/integration/autoland/rev/2af884f696e3
Remove bitfield in mozilla::detail::HashTable r=jandem
https://github.com/mozilla-firefox/firefox/commit/76504589695a
https://hg.mozilla.org/integration/autoland/rev/1d9b2325db58
Add MacroAssembler implementations of hash1/hash2/applyDoubleHash r=jandem
https://github.com/mozilla-firefox/firefox/commit/ad8bec08c3bf
https://hg.mozilla.org/integration/autoland/rev/f87d4af51d63
Implement MacroAssembler::lookupMFBT r=jandem
https://github.com/mozilla-firefox/firefox/commit/2c5382eac078
https://hg.mozilla.org/integration/autoland/rev/874197dbcfa1
Remove unused argument from emitPreBarrierFastPath r=jandem
https://github.com/mozilla-firefox/firefox/commit/94e910dc6229
https://hg.mozilla.org/integration/autoland/rev/4b3de371b2b9
Implement MacroAssembler::emitValueReadBarrierFastPath r=jandem,jonco
https://github.com/mozilla-firefox/firefox/commit/dbce81a8d69d
https://hg.mozilla.org/integration/autoland/rev/7000426011bd
Implement WeakMapObject::getObject inline r=jandem
https://github.com/mozilla-firefox/firefox/commit/81cf1665b1f5
https://hg.mozilla.org/integration/autoland/rev/f97ed7c9f411
Implement WeakMapObject::hasObject inline r=jandem

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/d06c70b12058
https://hg.mozilla.org/mozilla-central/rev/2af884f696e3
https://hg.mozilla.org/mozilla-central/rev/1d9b2325db58
https://hg.mozilla.org/mozilla-central/rev/f87d4af51d63
https://hg.mozilla.org/mozilla-central/rev/874197dbcfa1
https://hg.mozilla.org/mozilla-central/rev/4b3de371b2b9
https://hg.mozilla.org/mozilla-central/rev/7000426011bd
https://hg.mozilla.org/mozilla-central/rev/f97ed7c9f411