Closed
Bug 1370632
Opened 7 years ago
Closed 7 years ago
Move LookupRemoveIf to nsBaseHashtable?
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla55
| Tracking | Status | |
|---|---|---|
| firefox55 | --- | fixed |
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
Details
(Keywords: perf)
Attachments
(1 file, 3 obsolete files)
|
(deleted),
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
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...
|
Assignee | |
Comment 1•7 years ago
|
||
|
|
Assignee | |
Comment 2•7 years ago
|
||
WDYT?
|
|
Assignee | |
Comment 3•7 years ago
|
||
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
|
||
Comment 4•7 years ago
|
||
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.
|
|
Assignee | |
Comment 5•7 years ago
|
||
This should work if mAttributeCache never contain null values.
Otherwise, we need something like LookupForAdd().
Flags: needinfo?(bzbarsky)
|
|
Assignee | |
Comment 7•7 years ago
|
||
> mAttributeCache can't contain null values.
Great. I'll do the DOM changes separately in bug 1370700.
|
|
Assignee | |
Comment 8•7 years ago
|
||
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 9•7 years ago
|
||
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+
|
||
Comment 10•7 years ago
|
||
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
|
||
Comment 11•7 years ago
|
||
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•