Closed Bug 1480361 Opened 7 years ago Closed 7 years ago

Tweak handling of removed entries in HashTable::lookup()

Categories

(Core :: MFBT, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

Details

Attachments

(2 files)

There are two improvements here. - When we're just doing a lookup (i.e. aCollisionBit==0), we don't need to do any special handling of removed entries. (Inlining means that the removed entry code is entirely removed for lookups.) - When we're doing an insertion (i.e. aCollisionBit==sCollisionBit), we now stop adding collision markings once we find a removed entry, because they're unnecessary after that point. This change brings the code in alignment with PLDHashTable::SearchTable().
Comment on attachment 8996954 [details] [diff] [review] Tweak handling of removed entries in HashTable::lookup() Review of attachment 8996954 [details] [diff] [review]: ----------------------------------------------------------------- Nice find!
Attachment #8996954 - Flags: review?(luke) → review+
Again inspired by PLDHashTable, this makes things a bit clearer than passing 0 or sCollisionBit. It also guarantees more strongly that we'll end up with appropriately specialized code for the Add vs. NonAdd cases. (That guarantee isn't currently needed because the compiler inlines things sufficiently anyway, but it can't hurt.)
Attachment #8997177 - Flags: review?(luke)
Comment on attachment 8997177 [details] [diff] [review] Introduce HashTable::LookupReason Review of attachment 8997177 [details] [diff] [review]: ----------------------------------------------------------------- Yes, much clearer, thanks.
Attachment #8997177 - Flags: review?(luke) → review+
Pushed by nnethercote@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d9baec9cf420 Tweak handling of removed entries in HashTable::lookup(). r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/6c0410ddd1f6 Introduce HashTable::LookupReason. r=luke
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: