Bug 1865383 - Check slice budget when we start marking or sweeping r=sfink

The patch forces a budget check when we start marking or sweeping unless we
have just started a GC slice in the current state.

This turned out more complicated than expected because the change caused a test
that called gcslice(1) in a loop to not make any progress. This was because the
original stepAndForceCheck() method ate the single unit of work budget. The fix
was to remove the step part of this method. I don't think any of the callers
need it to step the budget as well.

Differential Revision: https://phabricator.services.mozilla.com/D193925
This commit is contained in:
Jon Coppeard 2023-11-20 16:28:11 +00:00
parent 72b45daca5
commit 2359bca10c
6 changed files with 23 additions and 12 deletions

View file

@ -112,14 +112,12 @@ class JS_PUBLIC_API SliceBudget {
counter -= steps;
}
// Do enough steps to force an "expensive" (time) check on the next call to
// isOverBudget. Useful when switching between major phases of an operation
// like a cycle collection.
void stepAndForceCheck() {
// Force an "expensive" (time) check on the next call to isOverBudget. Useful
// when switching between major phases of an operation like a cycle
// collection.
void forceCheck() {
if (isTimeBudget()) {
counter = 0;
} else {
counter--;
}
}

View file

@ -3000,6 +3000,13 @@ IncrementalProgress GCRuntime::markUntilBudgetExhausted(
AutoMajorGCProfilerEntry s(this);
if (initialState != State::Mark) {
sliceBudget.forceCheck();
if (sliceBudget.isOverBudget()) {
return NotFinished;
}
}
if (processTestMarkQueue() == QueueYielded) {
return NotFinished;
}

View file

@ -221,7 +221,7 @@ bool ParallelMarkTask::requestWork(AutoLockGC& lock) {
return false; // All other tasks are empty. We're finished.
}
budget.stepAndForceCheck();
budget.forceCheck();
if (budget.isOverBudget()) {
return false; // Over budget or interrupted.
}

View file

@ -2326,9 +2326,15 @@ IncrementalProgress GCRuntime::performSweepActions(SliceBudget& budget) {
}
#endif
if (initialState == State::Sweep &&
markDuringSweeping(gcx, budget) == NotFinished) {
return NotFinished;
if (initialState == State::Sweep) {
if (markDuringSweeping(gcx, budget) == NotFinished) {
return NotFinished;
}
} else {
budget.forceCheck();
if (budget.isOverBudget()) {
return NotFinished;
}
}
// Then continue running sweep actions.

View file

@ -1397,7 +1397,7 @@ bool CycleCollectedJSRuntime::TraceNativeGrayRoots(
TraceAdditionalNativeGrayRoots(aTracer);
mHolderIter.emplace(mJSHolders, aWhich);
aBudget.stepAndForceCheck();
aBudget.forceCheck();
} else {
// Holders may have been removed between slices, so we may need to update
// the iterator.

View file

@ -3506,7 +3506,7 @@ bool nsCycleCollector::Collect(CCReason aReason, ccIsManual aIsManual,
break;
}
if (continueSlice) {
aBudget.stepAndForceCheck();
aBudget.forceCheck();
continueSlice = !aBudget.isOverBudget();
}
} while (continueSlice);