1821659 - Fallible hashing (MovableCellHasher) performs unnecessary lookups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Jon Coppeard (:jonco)]

While looking into bug 1821107 I noticed that we do perform unnecessary hash table looks when using fallible hashing. Fixing this won't bring us to parity with V8 but should improve performance a little if we carry on using fallible hashing.

The problem is that the MovableCellHasher specializations of the hashHash and ensureHash methods do a hash table lookup but ignore the the result. The hashtable code then ends up doing the lookup again to get the hash code. This is a problem with the FallibleHashMethods interface.

Instead there should be a way for these methods to pass back any hash code found and have the hash table implementation use this.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1821659 - Part 1: Allow fallible hashing to return the hash code with maybeGetHash r?sfink

This is used where the hash table operation can fail early if we know there's
never been a hash code created for a lookup. If there has been a hash code
created, it's more efficient to return it though as otherwise we will always
query it straight away.

Therefore this renames |hasHash| to |maybeGetHash| and adds an output
parameter.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1821659 - Part 2: Return hash code from ensureHash too r?sfink

Similarly |ensureHash| should return the hash code it creates so we don't have
to look it up immediately.

Depends on D173122

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1821659 - Part 3: Tidy up and remove some unused code from the Zone class r?sfink

Comment 4

[Jon Coppeard (:jonco)]

This makes only a 1.5% improvement to the benchmark in bug 1821107, but it does strictly less work and may have more impact elsewhere.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66d03ada68ef
Part 1: Allow fallible hashing to return the hash code with maybeGetHash r=sfink
https://hg.mozilla.org/integration/autoland/rev/04c60244e367
Part 2: Return hash code from ensureHash too r=sfink
https://hg.mozilla.org/integration/autoland/rev/8744e107741d
Part 3: Tidy up and remove some unused code from the Zone class r=sfink

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/66d03ada68ef
https://hg.mozilla.org/mozilla-central/rev/04c60244e367
https://hg.mozilla.org/mozilla-central/rev/8744e107741d