Closed Bug 1370632 Opened 2 years ago Closed 2 years ago

Move LookupRemoveIf to nsBaseHashtable?

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mats, Assigned: mats)

References

Details

(Keywords: perf)

Attachments

(1 file, 3 obsolete files)

I wonder if we should move LookupRemoveIf() up to nsBaseHashtable
instead so that it can be used on other types of hashtables too?

I've made an example for nsDOMAttributeMap::DropAttribute, WIP patches
coming up...
Looks like it would be easy to save a hashtable lookup in nsDOMAttributeMap::GetAttribute too
if we had a LookupForAdd... or perhaps GetOrInsert() can handle this case?
http://searchfox.org/mozilla-central/rev/d441cb24482c2e5448accaf07379445059937080/dom/base/nsDOMAttributeMap.cpp#138,143
Moving LookupRemoveIf to nsBaseHashtable makes sense to me.

We have LookupForAdd/LookupOrAdd in nsClassHashtable; we should see about porting those over to nsRefPtrHashtable and/or nsBaseHashtable, as appropriate.
This should work if mAttributeCache never contain null values.
Otherwise, we need something like LookupForAdd().
Flags: needinfo?(bzbarsky)
mAttributeCache can't contain null values.
Flags: needinfo?(bzbarsky)
Blocks: 1370674
Blocks: 1370700
> mAttributeCache can't contain null values.

Great.  I'll do the DOM changes separately in bug 1370700.
This is a verbatim copy, with "base_type::" removed.
Assignee: nobody → mats
Attachment #8874947 - Attachment is obsolete: true
Attachment #8874948 - Attachment is obsolete: true
Attachment #8874956 - Attachment is obsolete: true
Attachment #8875034 - Flags: review?(nfroyd)
Comment on attachment 8875034 [details] [diff] [review]
Move LookupRemoveIf() to nsBaseHashtable instead so that it can be used on more hashtables types

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

Thanks!
Attachment #8875034 - Flags: review?(nfroyd) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c7be210a5c7
Move LookupRemoveIf() to nsBaseHashtable instead so that it can be used on more hashtables types.  r=froydnj
https://hg.mozilla.org/mozilla-central/rev/5c7be210a5c7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.