1848884 - Store scrambled hash values in atoms

Status: ASSIGNED

(Core :: JavaScript Engine, task, P2)

Description

[Steve Fink [:sfink] [:s:]]

We use the type HashNumber to represent a number of different values. I'll name them

• raw hashes: directly computed from the input
• scrambled hashes: passed through a scrambler that spreads out the entropy to make it more uniform
• prepared hashes: mfbt/HashTable.h reserves the bottom bit for collisions, and zero for a null value.
• entry hashes: a prepared hash with information in its collision bit.
• hash index: a hash value, possibly shifted or masked, used to do address computations.

Currently, we store raw hashes in atoms and therefore must re-scramble and re-prepare them before use. We could consider storing prepared hashes instead.

Comment 1

[Steve Fink [:sfink] [:s:]]

I implemented this, and honestly it didn't really make any difference in performance. We might still decide to land part of this stack purely for the preparatory work, which creates a new mozilla::KeyHash type to distinguish some of the different types of HashNumbers.

My try push did show significant improvements in stylebench on osx but I don't believe them. If I look at the actual data points, some of the subtests really did have results where all of the patched version did better than all of the unpatched, but that looks more like a chance difference between binaries to me.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Refactor deduplication hash computation.

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Split out a KeyHash

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Switch atoms to using JSAtom::computeHash

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Allow HashPolicy to define preparedHash() in place of hash()

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Allow FallibleHashPolicy to use prepared hashes

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Remove some duplication in derived HashPolicy definitions

Comment 8

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Switch atoms to storing a prepared hash

Comment 9

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1848884 - Do not re-scramble atom and symbol hash codes when using them for OrderedHashTable