Closed
Bug 1480361
Opened 7 years ago
Closed 7 years ago
Tweak handling of removed entries in HashTable::lookup()
Categories
(Core :: MFBT, enhancement)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
Attachments
(2 files)
1.56 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
4.48 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
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().
![]() |
Assignee | |
Comment 1•7 years ago
|
||
Attachment #8996954 -
Flags: review?(luke)
![]() |
||
Comment 2•7 years ago
|
||
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+
![]() |
Assignee | |
Comment 3•7 years ago
|
||
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 4•7 years ago
|
||
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
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d9baec9cf420
https://hg.mozilla.org/mozilla-central/rev/6c0410ddd1f6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•