Closed Bug 712170 Opened 13 years ago Closed 9 years ago

[CC] Don't add nsJSEventListeners to the CC graph that have no grandchildren

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: mccr8, Unassigned)

References

(Blocks 1 open bug)

Details

After bug 702036, nsJSEventListeners don't drag in quite as many nodes. But it is still fairly common to see hundreds of them pointing to a particular JSContext. If GetOutstandingRequests() is non-zero for the context, we root it. We should be able to take advantage of this at least some of the time to not add the event listeners to the graph. Are the event listeners held alive by the context? If so, we can just skip the listener if the context has an outstanding request. If not, we have to do some more detailed analysis (maybe it will only be okay to skip it if the context is the only out edge of the listener?). There is also the usual disclaimer that we can't skip the node if there are any script objects, but that doesn't seem common. We should also see how common it is for these contexts to have outstanding requests.
Blocks: 716598
This isn't quite right. I think the thing to look for is to see if the nsJSEventListener is necessarily not part of a cycle. They only have two children, an nsGlobalWindow and a JSContext. These children often have no children, in which case we don't need to add the nsJSEventListener to the graph. I'm looking at the latter in bug 705371.
Depends on: 705371
Summary: [CC] Don't add nsJSEventListeners to the CC graph that have contexts with outstanding requests? → [CC] Don't add nsJSEventListeners to the CC graph that have no grandchildren
The way to implement this is probably to add a CanSkip() to nsGlobalWindow, JSContext, then add one to nsJSEventListener that checks those of its two children. Hopefully they won't be very expensive.
Depends on: 716518
Not sure whether it is about this bug, but currently about 50% of my CC graph is nsJSEventListener objects. The only reason they are there is that xpconnect adds pretty much all the mJSHolders to the graph. I could add CanSkipThis to nsJSEventListener, but something needs to keep the cx alive.
Do the children of the event listeners have children? In graphs I saw, the global windows and JSContexts that were children of them mostly had no children and thus could be avoided themselves.
There is only mContext (since the js objects are marked black before CC). mContext has only mGlobalObjectRef and mContext, and mGlobalObjectRef has nothing. So, ok, checking whether mGlobalObjectRef is in CCGeneration or black should be enough.
https://tbpl.mozilla.org/?tree=Try&rev=3e698ab57508 removes jseventlisteners from the CC graph. Unfortunately I don't see much difference in CC times. (Though, I've been opening more and more tabs, currently 190+)
Hmm, or perhaps there is some improvement. Since after closing some tabs so that there are about 100~ tabs, I'm getting CC times 15-25ms (when there hasn't been page (un)loads).
Depends on: 718634
No longer blocks: 702032
CC graphs are pretty small now. I don't think this is a big deal any more.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.