зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1711128 - Revert to pre-1692308 GCReason handling r=pbone,smaug
Bug 1692308 made some changes to how GCReasons are reported. Previously, the reasons reported for slices depended on how the GC was triggered: internally triggered GCs would have the internal reason for their first slice and INTER_SLICE_GC for every later slice. PokeGC-triggered GCs would use the PokeGC reason for *all* slices. (Or if RunNextCollectorTimer was called before the GC began, its reason would get swapped in for the PokeGC one.) After bug 1692308, INTER_SLICE_GC was used for all non-initial slices. This change wasn't entirely intentional (part of it was). This patch reverts to the previous behavior, and in so doing adjusts the use of sScheduler.mMajorGCReason (it maintains the reason that we want to use for later slices, rather than clearing it when we get the go-ahead from the parent.) Note that this handling of GCReasons is intended to be temporary. I want to fix it properly in a later bug. Differential Revision: https://phabricator.services.mozilla.com/D115096
This commit is contained in:
Steve Fink
Родитель
976e686288
Коммит
c0e0fbb9aa
|
|
@ -146,11 +146,10 @@ class CCGCScheduler {
|
|||
void SetNeedsFullGC(bool aNeedGC = true) { mNeedsFullGC = aNeedGC; }
|
||||
|
||||
void SetWantMajorGC(JS::GCReason aReason) {
|
||||
if (mMajorGCReason == JS::GCReason::NO_REASON) {
|
||||
// If there was already a reason for doing a major GC, do not override it
|
||||
// with a newer reason. When looking at the profiler, the initial reason
|
||||
// is probably more useful to know than any followup reasons while things
|
||||
// are getting worse.
|
||||
if (aReason != JS::GCReason::DOM_WINDOW_UTILS ||
|
||||
mMajorGCReason == JS::GCReason::NO_REASON) {
|
||||
// DOM_WINDOW_UTILS is used for testing interprocess GC coordination and
|
||||
// so should not override a pre-existing reason.
|
||||
mMajorGCReason = aReason;
|
||||
}
|
||||
|
||||
|
|
@ -168,8 +167,7 @@ class CCGCScheduler {
|
|||
mNeedsGCAfterCC = true;
|
||||
}
|
||||
|
||||
void NoteReadyForMajorGC(JS::GCReason aReason) {
|
||||
mMajorGCReason = aReason;
|
||||
void NoteReadyForMajorGC() {
|
||||
mReadyForMajorGC = true;
|
||||
}
|
||||
|
||||
|
|
@ -181,6 +179,8 @@ class CCGCScheduler {
|
|||
}
|
||||
|
||||
void NoteGCEnd() {
|
||||
mMajorGCReason = JS::GCReason::NO_REASON;
|
||||
|
||||
mInIncrementalGC = false;
|
||||
mCCBlockStart = TimeStamp();
|
||||
mInIncrementalGC = false;
|
||||
|
|
@ -194,6 +194,27 @@ class CCGCScheduler {
|
|||
mLikelyShortLivingObjectsNeedingGC = 0;
|
||||
}
|
||||
|
||||
void NoteGCSliceEnd() {
|
||||
if (mMajorGCReason == JS::GCReason::NO_REASON) {
|
||||
// If the GC was internally triggered, then we won't have a reason set
|
||||
// and want to use INTER_SLICE_GC for every subsequent slice. If the GC
|
||||
// was triggered by PokeGC, then we will have a reason and want every
|
||||
// subsequent slice to inherit that reason (*unless*
|
||||
// RunNextCollectorTimer provides a new reason before the initial timer
|
||||
// fires, in which case the PokeGC reason will be replaced.)
|
||||
//
|
||||
// This behavior doesn't make all that much sense, but reverts to the
|
||||
// behavior before bug 1692308. A following patch will change the
|
||||
// behavior.
|
||||
mMajorGCReason = JS::GCReason::INTER_SLICE_GC;
|
||||
|
||||
// Also, internally-triggered GCs do not wait for the parent's permission
|
||||
// to proceed. This flag won't be checked during an incremental GC, but
|
||||
// it better reflects reality.
|
||||
mReadyForMajorGC = true;
|
||||
}
|
||||
}
|
||||
|
||||
// When we decide to do a cycle collection but we're in the middle of an
|
||||
// incremental GC, the CC is "locked out" until the GC completes -- unless
|
||||
// the wait is too long, and we decide to finish the incremental GC early.
|
||||
|
|
@ -327,7 +348,7 @@ class CCGCScheduler {
|
|||
|
||||
inline GCRunnerStep GetNextGCRunnerAction(TimeStamp aDeadline);
|
||||
|
||||
inline CCRunnerStep GetNextCCRunnerAction(TimeStamp aDeadline);
|
||||
inline CCRunnerStep AdvanceCCRunner(TimeStamp aDeadline);
|
||||
|
||||
// aStartTimeStamp : when the ForgetSkippable timer fired. This may be some
|
||||
// time ago, if an incremental GC needed to be finished.
|
||||
|
|
@ -369,11 +390,11 @@ class CCGCScheduler {
|
|||
bool mNeedsGCAfterCC = false;
|
||||
uint32_t mPreviousSuspectedCount = 0;
|
||||
|
||||
JS::GCReason mMajorGCReason = JS::GCReason::NO_REASON;
|
||||
|
||||
uint32_t mCleanupsSinceLastGC = UINT32_MAX;
|
||||
|
||||
public:
|
||||
JS::GCReason mMajorGCReason = JS::GCReason::NO_REASON;
|
||||
|
||||
uint32_t mCCollectedWaitingForGC = 0;
|
||||
uint32_t mCCollectedZonesWaitingForGC = 0;
|
||||
uint32_t mLikelyShortLivingObjectsNeedingGC = 0;
|
||||
|
|
@ -476,7 +497,7 @@ bool CCGCScheduler::ShouldScheduleCC() const {
|
|||
return IsCCNeeded(now);
|
||||
}
|
||||
|
||||
CCRunnerStep CCGCScheduler::GetNextCCRunnerAction(TimeStamp aDeadline) {
|
||||
CCRunnerStep CCGCScheduler::AdvanceCCRunner(TimeStamp aDeadline) {
|
||||
struct StateDescriptor {
|
||||
// When in this state, should we first check to see if we still have
|
||||
// enough reason to CC?
|
||||
|
|
@ -661,22 +682,14 @@ CCRunnerStep CCGCScheduler::GetNextCCRunnerAction(TimeStamp aDeadline) {
|
|||
|
||||
GCRunnerStep CCGCScheduler::GetNextGCRunnerAction(TimeStamp aDeadline) {
|
||||
if (InIncrementalGC()) {
|
||||
return {GCRunnerAction::GCSlice, JS::GCReason::INTER_SLICE_GC};
|
||||
return {GCRunnerAction::GCSlice, mMajorGCReason};
|
||||
}
|
||||
|
||||
if (mReadyForMajorGC) {
|
||||
GCRunnerStep step{GCRunnerAction::StartMajorGC, mMajorGCReason};
|
||||
mMajorGCReason = JS::GCReason::NO_REASON;
|
||||
return step;
|
||||
return {GCRunnerAction::StartMajorGC, mMajorGCReason};
|
||||
}
|
||||
|
||||
if (mMajorGCReason != JS::GCReason::NO_REASON) {
|
||||
GCRunnerStep step{GCRunnerAction::WaitToMajorGC, mMajorGCReason};
|
||||
mMajorGCReason = JS::GCReason::NO_REASON;
|
||||
return step;
|
||||
}
|
||||
|
||||
return {GCRunnerAction::None, JS::GCReason::NO_REASON};
|
||||
return {GCRunnerAction::WaitToMajorGC, mMajorGCReason};
|
||||
}
|
||||
|
||||
inline js::SliceBudget CCGCScheduler::ComputeForgetSkippableBudget(
|
||||
|
|
|
|||
|
|
@ -1599,7 +1599,8 @@ void nsJSContext::EndCycleCollectionCallback(CycleCollectorResults& aResults) {
|
|||
}
|
||||
|
||||
// static
|
||||
bool GCRunnerFired(TimeStamp aDeadline, void* /* aClosure */) {
|
||||
bool GCRunnerFired(TimeStamp aDeadline, void* aClosure) {
|
||||
MOZ_ASSERT(!aClosure, "Don't pass a closure to GCRunnerFired");
|
||||
MOZ_ASSERT(!sShuttingDown, "GCRunner still alive during shutdown");
|
||||
|
||||
GCRunnerStep step = sScheduler.GetNextGCRunnerAction(aDeadline);
|
||||
|
|
@ -1617,11 +1618,10 @@ bool GCRunnerFired(TimeStamp aDeadline, void* /* aClosure */) {
|
|||
}
|
||||
|
||||
nsJSContext::KillGCRunner();
|
||||
JS::GCReason reason = step.mReason;
|
||||
mbPromise->Then(
|
||||
GetMainThreadSerialEventTarget(), __func__,
|
||||
[reason](bool aIgnored) {
|
||||
sScheduler.NoteReadyForMajorGC(reason);
|
||||
[](bool aIgnored) {
|
||||
sScheduler.NoteReadyForMajorGC();
|
||||
// If a new runner was started, recreate it with a 0 delay. The new
|
||||
// runner will continue in idle time.
|
||||
nsJSContext::KillGCRunner();
|
||||
|
|
@ -1638,6 +1638,7 @@ bool GCRunnerFired(TimeStamp aDeadline, void* /* aClosure */) {
|
|||
|
||||
return true;
|
||||
}
|
||||
|
||||
case GCRunnerAction::StartMajorGC:
|
||||
case GCRunnerAction::GCSlice:
|
||||
break;
|
||||
|
|
@ -1722,7 +1723,7 @@ static bool CCRunnerFired(TimeStamp aDeadline) {
|
|||
// `Yield` in step.mYield.
|
||||
CCRunnerStep step;
|
||||
do {
|
||||
step = sScheduler.GetNextCCRunnerAction(aDeadline);
|
||||
step = sScheduler.AdvanceCCRunner(aDeadline);
|
||||
switch (step.mAction) {
|
||||
case CCRunnerAction::None:
|
||||
break;
|
||||
|
|
@ -1798,10 +1799,14 @@ void nsJSContext::RunNextCollectorTimer(JS::GCReason aReason,
|
|||
|
||||
RefPtr<IdleTaskRunner> runnable;
|
||||
if (sGCRunner) {
|
||||
// If the GC has already started, we will just run the next slice here and
|
||||
// this call will have no effect (the reason will be ignored, and the
|
||||
// trigger will be cleared at the end of the GC.)
|
||||
sScheduler.SetWantMajorGC(aReason);
|
||||
// To maintain pre-bug 1692308 GCReason handling: RunNextCollectorTimer
|
||||
// should override the PokeGC reason, but only if the first slice has not
|
||||
// happened yet. If there was no PokeGC reason (ie, this is an
|
||||
// internally-generated GC), then do not override the reason (which will be
|
||||
// INTER_SLICE_GC).
|
||||
if (!sScheduler.InIncrementalGC() && sScheduler.mMajorGCReason != JS::GCReason::INTER_SLICE_GC) {
|
||||
sScheduler.SetWantMajorGC(aReason);
|
||||
}
|
||||
sGCRunner->SetIdleDeadline(aDeadline);
|
||||
runnable = sGCRunner;
|
||||
} else {
|
||||
|
|
@ -2061,6 +2066,8 @@ static void DOMGCSliceCallback(JSContext* aCx, JS::GCProgress aProgress,
|
|||
break;
|
||||
|
||||
case JS::GC_SLICE_END:
|
||||
sScheduler.NoteGCSliceEnd();
|
||||
|
||||
sGCUnnotifiedTotalTime +=
|
||||
aDesc.lastSliceEnd(aCx) - aDesc.lastSliceStart(aCx);
|
||||
|
||||
|
|
|
|||
|
|
@ -120,7 +120,7 @@ void TestCC::TimerFires(int aNumSlices) {
|
|||
while (true) {
|
||||
SuspectMore(1000);
|
||||
TimeStamp idleDeadline = Now() + kOneSecond;
|
||||
step = mScheduler.GetNextCCRunnerAction(idleDeadline);
|
||||
step = mScheduler.AdvanceCCRunner(idleDeadline);
|
||||
// Should first see a series of ForgetSkippable actions.
|
||||
if (step.mAction != CCRunnerAction::ForgetSkippable ||
|
||||
step.mRemoveChildless != KeepChildless) {
|
||||
|
|
@ -132,16 +132,16 @@ void TestCC::TimerFires(int aNumSlices) {
|
|||
|
||||
while (step.mYield == Continue) {
|
||||
TimeStamp idleDeadline = Now() + kOneSecond;
|
||||
step = mScheduler.GetNextCCRunnerAction(idleDeadline);
|
||||
step = mScheduler.AdvanceCCRunner(idleDeadline);
|
||||
}
|
||||
EXPECT_EQ(step.mAction, CCRunnerAction::ForgetSkippable);
|
||||
EXPECT_EQ(step.mRemoveChildless, RemoveChildless);
|
||||
ForgetSkippable();
|
||||
|
||||
TimeStamp idleDeadline = Now() + kOneSecond;
|
||||
step = mScheduler.GetNextCCRunnerAction(idleDeadline);
|
||||
step = mScheduler.AdvanceCCRunner(idleDeadline);
|
||||
EXPECT_EQ(step.mAction, CCRunnerAction::CleanupContentUnbinder);
|
||||
step = mScheduler.GetNextCCRunnerAction(idleDeadline);
|
||||
step = mScheduler.AdvanceCCRunner(idleDeadline);
|
||||
EXPECT_EQ(step.mAction, CCRunnerAction::CleanupDeferred);
|
||||
|
||||
RunSlices(aNumSlices);
|
||||
|
|
@ -203,7 +203,7 @@ void TestIdleCC::RunSlice(TimeStamp aCCStartTime, TimeStamp aPrevSliceEnd,
|
|||
TimeStamp idleDeadline = Now() + kTenthSecond;
|
||||
|
||||
// The scheduler should request a CycleCollect slice.
|
||||
step = mScheduler.GetNextCCRunnerAction(idleDeadline);
|
||||
step = mScheduler.AdvanceCCRunner(idleDeadline);
|
||||
EXPECT_EQ(step.mAction, CCRunnerAction::CycleCollect);
|
||||
|
||||
// nsJSContext::RunCycleCollectorSlice
|
||||
|
|
@ -241,7 +241,7 @@ void TestNonIdleCC::RunSlice(TimeStamp aCCStartTime, TimeStamp aPrevSliceEnd,
|
|||
TimeStamp nullDeadline;
|
||||
|
||||
// The scheduler should tell us to run a slice of cycle collection.
|
||||
step = mScheduler.GetNextCCRunnerAction(nullDeadline);
|
||||
step = mScheduler.AdvanceCCRunner(nullDeadline);
|
||||
EXPECT_EQ(step.mAction, CCRunnerAction::CycleCollect);
|
||||
|
||||
// nsJSContext::RunCycleCollectorSlice
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче