A lot of time is spent in AddRef / Release during button.click() / checkbox.click(), especially when clearing the event target chain
Categories
(Core :: DOM: Events, defect)
Tracking
()
People
(Reporter: mstange, Assigned: smaug)
References
(Blocks 1 open bug)
Details
(Whiteboard: [sp3])
Attachments
(3 files)
The testcases in bug 1834003 spend a fair amount of time calling AddRef and Release on cycle collected objects. https://share.firefox.dev/3DaKDxI
The biggest offender is nsTArray<EventTargetChainItem>::ClearAndRetainStorage
. This function is called four times in total per call to .click()
, probably with identical event target chains: For click
, DOMActivate
, input
and change
. It spends all its time calling virtual Release
methods which spend their time in nsCycleCollectingAutoRefCnt::decr
.
Updated•1 year ago
|
Assignee | ||
Comment 1•1 year ago
|
||
Yes. We need to keep the event targets alive, so the chain must have strong references. And yes, in these particular test cases the chain might be the same but in general it isn't. The spec requires us to have separate event path (== chain) for each event.
But we should make AddRef/Release non-virtual in this case. Non-trivial change, since it requires moving refcnt to EventTarget, and some EventTargets use mRefCnt.incr/decr<NS_CycleCollectorSuspectUsingNursery>, and some mRefCt.incr/decr<NS_CycleCollectorSuspect3>.
The nursery is available on the main thread only currently, because accessing it was tried to be as fast as possible.
Another thing we could do in these nested events case is that there could be a flag (there should be space for one flag in nsWrapperCache's flags) whether the target is currently part of an event chain, and if so, we can use raw pointers in the nested chains. Not sure if that would help much though if we make AddRef/Release non-virtual.
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 2•1 year ago
|
||
The preliminary results look quite good
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=2cfd6144f271283f6a6964bcf50efcd7770a3d11&originalSignature=4586009&newSignature=4586009&framework=13&application=firefox&originalRevision=17fc117158fff8225831b449e60c3308c5c69951&page=1&showOnlyConfident=1
Svelte test for example spends lots of time in event dispatch
Assignee | ||
Comment 3•1 year ago
|
||
Assignee | ||
Comment 4•1 year ago
|
||
Depends on D187146
Assignee | ||
Comment 5•1 year ago
|
||
Depends on D187147
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Description
•