Improve ObjectGroup hash in TenureCountCache in 64 bit builds

RESOLVED FIXED in Firefox 51

Status

()

Core
JavaScript: GC
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
Created attachment 8778896 [details] [diff] [review]
fix-tenure-count-hash

Currently we use PointerHasher<ObjectGroup*, 3>::hash() to hash the ObjectGroup pointer, and use the result modulo the table size as an index.

This function shifts off the bottom 3 bits and combines the two 32-bit halves of the remaining word with XOR.  The table size is 16, so we then use the bottom four bits of that.

In 64 bit builds the bottom 4 bits of the ObjectGroup pointer are always zero since the size of the object is 48 bytes (it's 24 in 32 bit builds).  This means the bottom bit of the result comes only from bit 29 (32 - 3) of the input, which is almost always going to be the same.  This means we usually only use half the entries in the table (I originally found this because splay wasn't always pretenuring as expected).

Here's a patch to shift off 4 bits on 64 bit builds and to XOR in bits 8-4 of the remaining word which are more likely to be different.
Attachment #8778896 - Flags: review?(terrence)
Comment on attachment 8778896 [details] [diff] [review]
fix-tenure-count-hash

Review of attachment 8778896 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent! We put approximately zero thought into that once we got splay working twice in a row on Brian's laptop. I think I meant to take a closer look, but it obviously fell off the radar.
Attachment #8778896 - Flags: review?(terrence) → review+

Comment 2

a year ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/547340fb951d
Improve ObjectGroup hash in TenureCountCache in 64 bit builds r=terrence

Comment 3

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/547340fb951d
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.