Skip to content

Commit 473dfca

Browse files
committed
Bug 1836085 - Set nursery position to the end of the chunk when minor GC is requested r=jandem
This allows us to fold the check for this into the space check for nursery allocations. This change also means we don't trigger requested major GC on nursery allocations. We still trigger this via interrupts. This also separates the interrupt bits for minor and major GCs. Differential Revision: https://phabricator.services.mozilla.com/D179789
1 parent 868ad74 commit 473dfca

File tree

9 files changed

+86
-48
lines changed

9 files changed

+86
-48
lines changed

js/src/gc/Allocator.cpp

+28-18
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,18 @@ void* GCRuntime::tryNewNurseryCell(JSContext* cx, size_t thingSize,
120120
MOZ_ASSERT(!cx->zone()->isAtomsZone());
121121
MOZ_ASSERT(cx->zone()->allocKindInNursery(kind));
122122

123-
void* ptr = cx->nursery().allocateCell(site, thingSize, kind);
123+
Nursery& nursery = cx->nursery();
124+
void* ptr = nursery.allocateCell(site, thingSize, kind);
124125
if (ptr) {
125126
return ptr;
126127
}
127128

128129
if constexpr (allowGC) {
129130
if (!cx->suppressGC) {
130-
cx->runtime()->gc.minorGC(JS::GCReason::OUT_OF_NURSERY);
131+
JS::GCReason reason = nursery.minorGCRequested()
132+
? nursery.minorGCTriggerReason()
133+
: JS::GCReason::OUT_OF_NURSERY;
134+
cx->runtime()->gc.minorGC(reason);
131135

132136
// Exceeding gcMaxBytes while tenuring can disable the Nursery.
133137
if (cx->zone()->allocKindInNursery(kind)) {
@@ -164,6 +168,10 @@ template <AllowGC allowGC>
164168
/* static */
165169
void* GCRuntime::tryNewTenuredThing(JSContext* cx, AllocKind kind,
166170
size_t thingSize) {
171+
if constexpr (allowGC) {
172+
gcIfNeededAtAllocation(cx);
173+
}
174+
167175
// Bump allocate in the arena's current free-list span.
168176
Zone* zone = cx->zone();
169177
void* ptr = zone->arenas.freeLists().allocate(kind);
@@ -219,23 +227,30 @@ void GCRuntime::attemptLastDitchGC(JSContext* cx) {
219227
lastLastDitchTime = mozilla::TimeStamp::Now();
220228
}
221229

230+
#ifdef DEBUG
231+
static bool IsAtomsZoneKind(AllocKind kind) {
232+
return kind == AllocKind::ATOM || kind == AllocKind::FAT_INLINE_ATOM ||
233+
kind == AllocKind::SYMBOL;
234+
}
235+
#endif
236+
222237
template <AllowGC allowGC>
223238
bool GCRuntime::checkAllocatorState(JSContext* cx, AllocKind kind) {
224239
MOZ_ASSERT_IF(cx->zone()->isAtomsZone(),
225-
kind == AllocKind::ATOM || kind == AllocKind::FAT_INLINE_ATOM ||
226-
kind == AllocKind::SYMBOL || kind == AllocKind::JITCODE ||
227-
kind == AllocKind::SCOPE);
228-
MOZ_ASSERT_IF(!cx->zone()->isAtomsZone(),
229-
kind != AllocKind::ATOM && kind != AllocKind::FAT_INLINE_ATOM);
230-
MOZ_ASSERT(!JS::RuntimeHeapIsBusy());
240+
IsAtomsZoneKind(kind) || kind == AllocKind::JITCODE);
241+
MOZ_ASSERT_IF(!cx->zone()->isAtomsZone(), !IsAtomsZoneKind(kind));
231242

232243
if constexpr (allowGC) {
233244
// Crash if we could perform a GC action when it is not safe.
234245
if (!cx->suppressGC) {
235246
cx->verifyIsSafeToGC();
236-
}
237247

238-
gcIfNeededAtAllocation(cx);
248+
#ifdef JS_GC_ZEAL
249+
if (needZealousGC()) {
250+
runDebugGC();
251+
}
252+
#endif
253+
}
239254
}
240255

241256
// For testing out of memory conditions.
@@ -251,17 +266,12 @@ bool GCRuntime::checkAllocatorState(JSContext* cx, AllocKind kind) {
251266
return true;
252267
}
253268

269+
/* static */
254270
inline void GCRuntime::gcIfNeededAtAllocation(JSContext* cx) {
255-
#ifdef JS_GC_ZEAL
256-
if (needZealousGC()) {
257-
runDebugGC();
258-
}
259-
#endif
260-
261271
// Invoking the interrupt callback can fail and we can't usefully
262272
// handle that here. Just check in case we need to collect instead.
263-
if (cx->hasAnyPendingInterrupt()) {
264-
gcIfRequested();
273+
if (cx->hasPendingInterrupt(InterruptReason::MajorGC)) {
274+
cx->runtime()->gc.gcIfRequested();
265275
}
266276
}
267277

js/src/gc/GC.cpp

+1-13
Original file line numberDiff line numberDiff line change
@@ -1650,18 +1650,7 @@ void GCRuntime::requestMajorGC(JS::GCReason reason) {
16501650
}
16511651

16521652
majorGCTriggerReason = reason;
1653-
rt->mainContextFromAnyThread()->requestInterrupt(InterruptReason::GC);
1654-
}
1655-
1656-
void Nursery::requestMinorGC(JS::GCReason reason) const {
1657-
MOZ_ASSERT(CurrentThreadCanAccessRuntime(runtime()));
1658-
1659-
if (minorGCRequested()) {
1660-
return;
1661-
}
1662-
1663-
minorGCTriggerReason_ = reason;
1664-
runtime()->mainContextFromOwnThread()->requestInterrupt(InterruptReason::GC);
1653+
rt->mainContextFromAnyThread()->requestInterrupt(InterruptReason::MajorGC);
16651654
}
16661655

16671656
bool GCRuntime::triggerGC(JS::GCReason reason) {
@@ -4596,7 +4585,6 @@ void GCRuntime::collectNursery(JS::GCOptions options, JS::GCReason reason,
45964585

45974586
gcstats::AutoPhase ap(stats(), phase);
45984587

4599-
nursery().clearMinorGCRequest();
46004588
nursery().collect(options, reason);
46014589
MOZ_ASSERT(nursery().isEmpty());
46024590

js/src/gc/GCRuntime.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ class GCRuntime {
668668
const AutoLockGC& lock);
669669

670670
// Allocator internals
671-
void gcIfNeededAtAllocation(JSContext* cx);
671+
static void gcIfNeededAtAllocation(JSContext* cx);
672672
static void* refillFreeList(JSContext* cx, AllocKind thingKind);
673673
void attemptLastDitchGC(JSContext* cx);
674674
#ifdef DEBUG

js/src/gc/Nursery.cpp

+36-1
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ js::Nursery::Nursery(GCRuntime* gc)
229229
reportPretenuring_(false),
230230
reportPretenuringThreshold_(0),
231231
minorGCTriggerReason_(JS::GCReason::NO_REASON),
232+
prevPosition_(0),
232233
hasRecentGrowthData(false),
233234
smoothedTargetSize(0.0) {
234235
const char* env = getenv("MOZ_NURSERY_STRINGS");
@@ -539,8 +540,8 @@ inline void* js::Nursery::allocate(size_t size) {
539540
MOZ_ASSERT(CurrentThreadCanAccessRuntime(runtime()));
540541
MOZ_ASSERT_IF(currentChunk_ == currentStartChunk_,
541542
position() >= currentStartPosition_);
542-
MOZ_ASSERT(position() % CellAlignBytes == 0);
543543
MOZ_ASSERT(size % CellAlignBytes == 0);
544+
MOZ_ASSERT(position() % CellAlignBytes == 0);
544545

545546
if (MOZ_UNLIKELY(currentEnd() < position() + size)) {
546547
return moveToNextChunkAndAllocate(size);
@@ -556,6 +557,12 @@ inline void* js::Nursery::allocate(size_t size) {
556557
}
557558

558559
void* Nursery::moveToNextChunkAndAllocate(size_t size) {
560+
if (minorGCRequested()) {
561+
// If a minor GC was requested then fail the allocation. The collection is
562+
// then run in GCRuntime::tryNewNurseryCell.
563+
return nullptr;
564+
}
565+
559566
MOZ_ASSERT(currentEnd() < position() + size);
560567

561568
unsigned chunkno = currentChunk_ + 1;
@@ -1124,6 +1131,15 @@ void js::Nursery::collect(JS::GCOptions options, JS::GCReason reason) {
11241131
JSRuntime* rt = runtime();
11251132
MOZ_ASSERT(!rt->mainContextFromOwnThread()->suppressGC);
11261133

1134+
if (minorGCRequested()) {
1135+
MOZ_ASSERT(position_ == chunk(currentChunk_).end());
1136+
position_ = prevPosition_;
1137+
prevPosition_ = 0;
1138+
minorGCTriggerReason_ = JS::GCReason::NO_REASON;
1139+
rt->mainContextFromOwnThread()->clearPendingInterrupt(
1140+
InterruptReason::MinorGC);
1141+
}
1142+
11271143
if (!isEnabled() || isEmpty()) {
11281144
// Our barriers are not always exact, and there may be entries in the
11291145
// storebuffer even when the nursery is disabled or empty. It's not safe
@@ -1584,6 +1600,25 @@ bool js::Nursery::registerMallocedBuffer(void* buffer, size_t nbytes) {
15841600
return true;
15851601
}
15861602

1603+
void Nursery::requestMinorGC(JS::GCReason reason) {
1604+
MOZ_ASSERT(reason != JS::GCReason::NO_REASON);
1605+
MOZ_ASSERT(CurrentThreadCanAccessRuntime(runtime()));
1606+
MOZ_ASSERT(isEnabled());
1607+
1608+
if (minorGCRequested()) {
1609+
return;
1610+
}
1611+
1612+
// Set position to end of chunk to block further allocation.
1613+
MOZ_ASSERT(prevPosition_ == 0);
1614+
prevPosition_ = position_;
1615+
position_ = chunk(currentChunk_).end();
1616+
1617+
minorGCTriggerReason_ = reason;
1618+
runtime()->mainContextFromOwnThread()->requestInterrupt(
1619+
InterruptReason::MinorGC);
1620+
}
1621+
15871622
size_t Nursery::sizeOfMallocedBuffers(
15881623
mozilla::MallocSizeOf mallocSizeOf) const {
15891624
size_t total = 0;

js/src/gc/Nursery.h

+6-8
Original file line numberDiff line numberDiff line change
@@ -313,15 +313,12 @@ class alignas(TypicalCacheLineSize) Nursery {
313313
return pretenuringNursery.addressOfAllocatedSites();
314314
}
315315

316-
void requestMinorGC(JS::GCReason reason) const;
316+
void requestMinorGC(JS::GCReason reason);
317317

318318
bool minorGCRequested() const {
319319
return minorGCTriggerReason_ != JS::GCReason::NO_REASON;
320320
}
321321
JS::GCReason minorGCTriggerReason() const { return minorGCTriggerReason_; }
322-
void clearMinorGCRequest() {
323-
minorGCTriggerReason_ = JS::GCReason::NO_REASON;
324-
}
325322

326323
bool shouldCollect() const;
327324
bool isNearlyFull() const;
@@ -419,10 +416,11 @@ class alignas(TypicalCacheLineSize) Nursery {
419416
bool reportPretenuring_;
420417
size_t reportPretenuringThreshold_;
421418

422-
// Whether and why a collection of this nursery has been requested. This is
423-
// mutable as it is set by the store buffer, which otherwise cannot modify
424-
// anything in the nursery.
425-
mutable JS::GCReason minorGCTriggerReason_;
419+
// Whether and why a collection of this nursery has been requested. When this
420+
// happens |prevPosition_| is set to the current position and |position_| set
421+
// to the end of the chunk to force the next allocation to fail.
422+
JS::GCReason minorGCTriggerReason_;
423+
uintptr_t prevPosition_;
426424

427425
// Profiling data.
428426

js/src/gc/StoreBuffer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ void StoreBuffer::GenericBuffer::trace(JSTracer* trc) {
7777
}
7878
}
7979

80-
StoreBuffer::StoreBuffer(JSRuntime* rt, const Nursery& nursery)
80+
StoreBuffer::StoreBuffer(JSRuntime* rt, Nursery& nursery)
8181
: lock_(mutexid::StoreBuffer),
8282
bufferVal(this, JS::GCReason::FULL_VALUE_BUFFER),
8383
bufStrCell(this, JS::GCReason::FULL_CELL_PTR_STR_BUFFER),

js/src/gc/StoreBuffer.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ class StoreBuffer {
439439
GenericBuffer bufferGeneric;
440440

441441
JSRuntime* runtime_;
442-
const Nursery& nursery_;
442+
Nursery& nursery_;
443443

444444
bool aboutToOverflow_;
445445
bool enabled_;
@@ -453,7 +453,7 @@ class StoreBuffer {
453453
bool markingNondeduplicatable;
454454
#endif
455455

456-
explicit StoreBuffer(JSRuntime* rt, const Nursery& nursery);
456+
explicit StoreBuffer(JSRuntime* rt, Nursery& nursery);
457457
[[nodiscard]] bool enable();
458458

459459
void disable();

js/src/vm/JSContext.h

+6-4
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,11 @@ bool CurrentThreadIsParseThread();
141141
#endif
142142

143143
enum class InterruptReason : uint32_t {
144-
GC = 1 << 0,
145-
AttachIonCompilations = 1 << 1,
146-
CallbackUrgent = 1 << 2,
147-
CallbackCanWait = 1 << 3,
144+
MinorGC = 1 << 0,
145+
MajorGC = 1 << 1,
146+
AttachIonCompilations = 1 << 2,
147+
CallbackUrgent = 1 << 3,
148+
CallbackCanWait = 1 << 4,
148149
};
149150

150151
enum class ShouldCaptureStack { Maybe, Always };
@@ -854,6 +855,7 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext,
854855
bool hasPendingInterrupt(js::InterruptReason reason) const {
855856
return interruptBits_ & uint32_t(reason);
856857
}
858+
void clearPendingInterrupt(js::InterruptReason reason);
857859

858860
// For JIT use. Points to the inlined ICScript for a baseline script
859861
// being invoked as part of a trial inlining. Contains nullptr at

js/src/vm/Runtime.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,11 @@ bool JSContext::handleInterrupt() {
488488
return true;
489489
}
490490

491+
void JSContext::clearPendingInterrupt(js::InterruptReason reason) {
492+
// Interrupt bit have already been cleared.
493+
interruptBits_ &= ~uint32_t(reason);
494+
}
495+
491496
bool JSRuntime::setDefaultLocale(const char* locale) {
492497
if (!locale) {
493498
return false;

0 commit comments

Comments
 (0)