Closed Bug 1721549 Opened 3 years ago Closed 3 years ago

Provide some GC reasons for embedders to use

Categories

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

task

Tracking

()

RESOLVED FIXED
92 Branch
Tracking Status
firefox-esr91 91+ fixed
firefox91 --- wontfix
firefox92 --- fixed

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(2 files)

We used to have some reserved GC reasons for this purpose but we gradually used them all up. At the moment embedders seem to be using Firefox-specific reasons, which has the risk that they will trigger some special behaviour that they didn't intend.

The current behaviours that we attach to reasons are:

  • MEM_PRESSURE and USER_INACTIVE: always compact in shrinking GCs, even if animating or incremental
  • anything with SHUTDOWN in it may be treated specially, e.g. by trying harder to clean everything up
  • they zeal mode test function deterministicgc() can disable certain GC reasons from running based on whether the reason is one that is triggered deterministically

We should add some reserved reasons for embedders that we don't attach special meanings too.

Assignee: nobody → jcoppeard
Status: NEW → ASSIGNED
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4a4059ea960e
Provide some GC reasons for embedders to use r=sfink
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch

Comment on attachment 9232332 [details]
Bug 1721549 - Provide some GC reasons for embedders to use r?sfink

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Allows embedders to avoid subtle bugs, so should go into the ESR that embedders target.
  • User impact if declined: Embedders might find subtle GC bugs when defining their own GC reasons to override Firefox's GC reasons, since some of those are treated specially by the JS engine.
  • Fix Landed on Version: 92
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Firefox doesn't use these embedder-only definitions.
  • String or UUID changes made by this patch: None.
Attachment #9232332 - Flags: approval-mozilla-esr91?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Since this would be useful let's add it.

Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e6d843aac93
Add JS::GCReason::FIRST_RESERVED_REASON r=sfink
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED

Comment on attachment 9232332 [details]
Bug 1721549 - Provide some GC reasons for embedders to use r?sfink

NPOTB for Firefox, helpful for embedders. Approved for 91.0esr RC2.

Attachment #9232332 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9232802 - Flags: approval-mozilla-esr91+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: