1376126 - CycleCollectedJSRuntime::RemoveJSHolder does unnecessary hashtable lookups

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement, P2)

Comment 1

[Mats Palmgren (inactive)]

Attachment: Use Lookup() entry instead of Get+Remove in CycleCollectedJSRuntime::RemoveJSHolder to avoid unnecessary hashtable lookups

I'm assuming that the Trace call can't change the mJSHolders hashtable.
We iterate mJSHolders in some places and call Trace so it seems
it should hold.
http://searchfox.org/mozilla-central/rev/77b256dc98efb93f1d118db456f442a09bba77c2/xpcom/base/CycleCollectedJSRuntime.cpp#780
http://searchfox.org/mozilla-central/rev/77b256dc98efb93f1d118db456f442a09bba77c2/xpcom/base/CycleCollectedJSRuntime.cpp#1044

Looks green on Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a729c616fe2523e055d6f0f4358e4aed005d79ed

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8881064 [details] [diff] [review]
Use Lookup() entry instead of Get+Remove in CycleCollectedJSRuntime::RemoveJSHolder to avoid unnecessary hashtable lookups

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

For avoidance of doubt: please wait for mccr8's feedback before landing.  Thanks!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4692d0721273
Use Lookup() entry instead of Get+Remove in CycleCollectedJSRuntime::RemoveJSHolder to avoid unnecessary hashtable lookups.  r=froydnj

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4692d0721273