Add to PLDHashTable the ability to remove an already-found entry

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(8 attachments)

(This is a slightly modified version of bug 848609.)

A very common use case for a hash table is this:

(1) lookup item
(2) do something with it
(3) remove it

With pldhash, for step (3) you either have to Remove(), which repeats the search, or use RawRemove(), which does not subsequently shrink the table if its capacity gets too low.

This bug is about adding a new function that takes an already found entry (like RawRemove()) but also shrinks the table afterwards if appropriate (like Remove()). And then using it when appropriate.
This patch also consolidates the shrink handling so it's now entirely within
ShrinkIfAppropriate().
Attachment #8658009 - Flags: review?(nfroyd)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
This assumes that it's safe to possibly shrink the table after the removal,
i.e. there are no surprising subtleties with how this table is managed.
Attachment #8658010 - Flags: review?(mcmanus)
This assumes that it's safe to possibly shrink the table after the removal,
i.e. there are no surprising subtleties with how this table is managed.
Attachment #8658011 - Flags: review?(bzbarsky)
This assumes that it's safe to possibly shrink the table after the removal,
i.e. there are no surprising subtleties with how this table is managed.
Attachment #8658012 - Flags: review?(bzbarsky)
This avoids repeating the hash table search in order to remove the entry.
Attachment #8658014 - Flags: review?(dkeeler)
This avoids repeating the hash table search in order to remove the entry.
Attachment #8658015 - Flags: review?(bzbarsky)
This avoids repeating the hash table search in order to remove the entry.
Attachment #8658016 - Flags: review?(nfroyd)
This avoids repeating the hash table search in order to remove a CC graph
entry, which is good because it's a common operation.
Attachment #8658017 - Flags: review?(continuation)
Attachment #8658017 - Flags: review?(continuation) → review+
Attachment #8658010 - Flags: review?(mcmanus) → review+
Attachment #8658009 - Flags: review?(nfroyd) → review+
Attachment #8658016 - Flags: review?(nfroyd) → review+
Comment on attachment 8658011 [details] [diff] [review]
(part 3) - Avoid PL_DHashTableRawRemove() in nsDocument

r=me
Attachment #8658011 - Flags: review?(bzbarsky) → review+
Comment on attachment 8658012 [details] [diff] [review]
(part 4) - Avoid PL_DHashTableRawRemove() in nsPropertyTable

r=me
Attachment #8658012 - Flags: review?(bzbarsky) → review+
Comment on attachment 8658015 [details] [diff] [review]
(part 6) - Use PLDHashTable::RemoveEntry() in XULDocument

r=me
Attachment #8658015 - Flags: review?(bzbarsky) → review+
Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cda97f7f4516

(Ignore all the red W tests in that push. There was some problem with them when I did the push.)
Blocks: 1209351
You need to log in before you can comment on or make changes to this bug.