Closed Bug 1376936 Opened 3 years ago Closed 3 years ago

nsContentLists should stop observing mutations when their refcnt drops to 0

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: smaug, Assigned: smaug)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

nsContentLists should stop observing mutations when their refcnt drops to 0.
This way they wouldn't affect DOM mutation performance while waiting to be deleted.
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=081c51f7e00853fc4e8880893cff60e42d804a12
remote: recorded changegroup in replication log in 0.115s
Attachment #8881975 - Flags: review?(ehsan)
Comment on attachment 8881975 [details] [diff] [review]
contentlist_last_release.diff

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

We do something very similar in nsContentList::NodeWillBeDestroyed() and it would be nice to share code.  It seems like since we're basically not going to get any new notifications in that case, we can have that function call LastRelease() directly.  If you agree, please consider making that change.

Thanks!
Attachment #8881975 - Flags: review?(ehsan) → review+
I was thinking that... 
though, ok, I could add a helper.
Attached patch contentlist_last_release_2.diff (obsolete) — Splinter Review
er, we don't want to call RemoveMutationObserver in NodeWillBeDestroyed.
I think it is good enough to keep those methods close enough.
Attachment #8882127 - Attachment is obsolete: true
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cec50535e2c
nsContentLists should stop observing mutations when their refcnt drops to 0, r=ehsan
Oh right there may be notifications for other nodes...  I was confused, sorry about that!
This is a profile after this patch: http://bit.ly/2tox8YY

Compare to a profile before from bug 1376936: http://bit.ly/2s0sgFB

This has cut down the cost of MatchNameAttribute roughly by about half in this profile it seems!
https://hg.mozilla.org/mozilla-central/rev/0cec50535e2c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.