Remove AutoSuppressNurseryCellAlloc
Categories
(Core :: JavaScript: GC, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox109 | --- | fixed |
People
(Reporter: arai, Assigned: tcampbell)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [sp3])
Attachments
(5 files)
bug 1745662 is going to remove the last non-test consumer of AutoSuppressNurseryCellAlloc
,
and the last test consumer of AutoSuppressNurseryCellAlloc
has the following comment:
// Now create forcibly-tenured versions of each of these string types. Note
// that this is best-effort; if nursery strings are disabled, or we GC
// midway through here, then we may end up with fewer nursery strings than
// desired. Also, some types of strings are not nursery-allocatable, so
// this will always produce some number of redundant strings.
gc::AutoSuppressNurseryCellAlloc suppress(cx);
So, if this isn't something guaranteed, we'd better removing it.
Reporter | ||
Comment 1•3 years ago
|
||
There are some comments that mentions off-thread allocation, that no longer happens,
and that's related to AutoSuppressNurseryCellAlloc
.
template <AllowGC allowGC /* = CanGC */>
JSString* js::AllocateStringImpl(JSContext* cx, AllocKind kind, size_t size,
InitialHeap heap) {
...
// Off-thread alloc cannot trigger GC or make runtime assertions.
if (cx->isNurseryAllocSuppressed()) {
template <AllowGC allowGC /* = CanGC */>
JS::BigInt* js::AllocateBigInt(JSContext* cx, InitialHeap heap) {
...
// Off-thread alloc cannot trigger GC or make runtime assertions.
if (cx->isNurseryAllocSuppressed()) {
template <js::AllowGC allowGC, typename CharT>
MOZ_ALWAYS_INLINE JSLinearString* JSLinearString::new_(
JSContext* cx, js::UniquePtr<CharT[], JS::FreePolicy> chars, size_t length,
js::gc::InitialHeap heap) {
...
if (!str->isTenured()) {
...
} else {
// This can happen off the main thread for the atoms zone.
the last one might be unrelated to AutoSuppressNurseryCellAlloc
, but still looks removable.
Reporter | ||
Comment 2•3 years ago
|
||
or, at least we could make it debug-only, so that the non-debug build doesn't have the branch
Reporter | ||
Updated•3 years ago
|
Reporter | ||
Comment 3•3 years ago
|
||
Depends on D133618
Assignee | ||
Comment 4•2 years ago
|
||
I was looking to fix this as well. Based on comments, it sounds like sfink doesn't need for the StructuredClone case.
I am able to refactor FillWithRepresentatives to simply pass gc::InitialHeap
to have the same effect as existing code.
Assignee | ||
Comment 5•2 years ago
|
||
Use the gc::InitialHeap parameters for string allocation functions instead to
guide strings as nursery vs tenured. The AutoSuppressNurseryCellAlloc mechanism
adds complexity to the GC that should probably be removed.
Assignee | ||
Comment 6•2 years ago
|
||
There shouldn't be any GC allocations any more. This looks like an minor
oversight that snuck in during a rebase.
Depends on D164299
Assignee | ||
Comment 7•2 years ago
|
||
Add an internal API to be able to specify the gc::InitialHeap instead.
Depends on D164300
Assignee | ||
Comment 8•2 years ago
|
||
This lets us remove some code from the various object and string allocation
paths.
Depends on D164301
Comment 10•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2f2a278c67a6
https://hg.mozilla.org/mozilla-central/rev/8baadad13803
https://hg.mozilla.org/mozilla-central/rev/fa48ad694a00
https://hg.mozilla.org/mozilla-central/rev/099904436a20
Updated•2 years ago
|
Updated•2 years ago
|
Description
•