1903034 - Drop JS objects sooner for promise callbacks

Status: RESOLVED DUPLICATE of bug 1953167

(Core :: DOM: Bindings (WebIDL), enhancement)

Description

[Jon Coppeard (:jonco)]

For promise callback objects, we clear their references to JS GC things after the callback has happened so as to not keep these alive longer than necessary. This is possible because these callbacks can only be run once.

This happens here: https://searchfox.org/mozilla-central/source/xpcom/base/CycleCollectedJSContext.cpp#212-217

Currently we leave the callback object in the JS holders table until it is destroyed. Removing it earlier would help with cases where frequent uses of promises results in this table growing large and increasing GC time.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1903034 - Call DropJSObjects after a promise callback has been called r?mccr8

For promise callbacks we call CallbackObject::Reset after the callback has been
called to clear JS references and avoid keeping JS GC things around longer than
necessary (promise callbacks can only be called once).

We don't need to keep the callback object in the JS holders table after this
point.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f342fbf7ef65
Call DropJSObjects after a promise callback has been called r=mccr8

Comment 3

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/f342fbf7ef65

Comment 4

[Mayank Bansal]

Could this have lead to a 10% improvement on AWFY-Jetstream2-async-fs-worst ?

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Mayank Bansal from comment #4)
Looks like that subtest makes heavy uses of promises, so I'd say yes.

Comment 6

[Noemi Erli[:noemi_erli]]

Reopening since this was backed out on request for causing Bug 1904810
Backout link: https://hg.mozilla.org/integration/autoland/rev/9663399d8f74eedee18666f60b7325fc8d1f3d26

Comment 7

[Serban Stanca [:SerbanS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/9663399d8f74eedee18666f60b7325fc8d1f3d26

Comment 8

[Donal Meehan [:dmeehan]]

Backout merged to beta: hg.mozilla.org/releases/mozilla-beta/rev/23e67191d0da0174665ab3a58f9ebf134f274f8a

Comment 9

[BugBot [:suhaib / :marco]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:jonco, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 10

[Jon Coppeard (:jonco)]

This change has happened as part of bug 1953167.