nsContentLists should stop observing mutations when their refcnt drops to 0

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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.

Updated

2 years ago
(Assignee)

Comment 1

2 years ago
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 2

2 years ago
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+
(Assignee)

Comment 3

2 years ago
I was thinking that... 
though, ok, I could add a helper.
(Assignee)

Comment 4

2 years ago
(Assignee)

Comment 5

2 years ago
er, we don't want to call RemoveMutationObserver in NodeWillBeDestroyed.
I think it is good enough to keep those methods close enough.
(Assignee)

Updated

2 years ago
Attachment #8882127 - Attachment is obsolete: true

Comment 6

2 years ago
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

Comment 7

2 years ago
Oh right there may be notifications for other nodes...  I was confused, sorry about that!

Comment 8

2 years ago
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!

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0cec50535e2c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.