Open Bug 1779767 Opened 2 years ago Updated 7 months ago

Preserved DOM reflectors always get tenured

Categories

(Core :: JavaScript: GC, task, P2)

task

Tracking

()

People

(Reporter: jandem, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached file Test case

See the attached test case. Profiling this with the Gecko profiler, it reports a 100% nursery promotion rate. I think this happens because we add a store buffer entry for preserved-wrappers. We probably want to treat this more like a weak pointer if there are no other references to the DOM node.

This is likely contributing to the high promotion rate on the matrix-react benchmark.

Assignee: nobody → jcoppeard

(In reply to Jan de Mooij [:jandem] from comment #0)
The idea would be to only trace perserved nursery wrappers if the ref count of the DOM node was > 1.

Andrew does that sound reasonable? If so, is there a way to get this ref count given that it's not part of nsWrapperCache?

Flags: needinfo?(continuation)

Maybe Peter or Olli can get to this before me.

Flags: needinfo?(peterv)
Flags: needinfo?(bugs)

Sounds rather unlikely that there would be lots of DOM nodes with ref count == 1. Usually nodes are part of at least some small subtree.

Flags: needinfo?(bugs)

(In reply to Olli Pettay [:smaug] from comment #3)

Sounds rather unlikely that there would be lots of DOM nodes with ref count == 1. Usually nodes are part of at least some small subtree.

Can you think of a way to handle that? The problem here is that React attaches huge object graphs to DOM nodes and we're currently forcing these to get tenured completely.

A DOM node with a preserved wrapper will never get freed until we run the CC, so I think tenuring these objects is reasonable. You'd need to add some kind of mechanism to minor GCs that understands DOM nodes enough to do a "mini cycle collection". I guess checking that the refcount is 1 is such a thing, but maybe you still want to unpreserve the wrapper?

Does the React benchmark really create nodes with zero other references besides the wrapper?

I know Olli has added some heuristics before to deal with DOM trees allocated with inner html that looks at the entire tree and sees if there are any external references. Though in this case you'd also want to be okay with references from wrappers, sometimes.

Flags: needinfo?(continuation)
Severity: normal → S3
Flags: needinfo?(peterv)
Priority: -- → P2
Assignee: jcoppeard → nobody
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: