return
}
- traced := false
+ // This extremely verbose boolean indicates whether we've
+ // entered mark assist from the perspective of the tracer.
+ //
+ // In the old tracer, this is just before we call gcAssistAlloc1
+ // *and* tracing is enabled. Because the old tracer doesn't
+ // do any extra tracking, we need to be careful to not emit an
+ // "end" event if there was no corresponding "begin" for the
+ // mark assist.
+ //
+ // In the new tracer, this is just before we call gcAssistAlloc1
+ // *regardless* of whether tracing is enabled. This is because
+ // the new tracer allows for tracing to begin (and advance
+ // generations) in the middle of a GC mark phase, so we need to
+ // record some state so that the tracer can pick it up to ensure
+ // a consistent trace result.
+ //
+ // TODO(mknyszek): Hide the details of inMarkAssist in tracer
+ // functions and simplify all the state tracking. This is a lot.
+ enteredMarkAssistForTracing := false
retry:
if gcCPULimiter.limiting() {
// If the CPU limiter is enabled, intentionally don't
// assist to reduce the amount of CPU time spent in the GC.
- if traced {
+ if enteredMarkAssistForTracing {
trace := traceAcquire()
if trace.ok() {
trace.GCMarkAssistDone()
+ // Set this *after* we trace the end to make sure
+ // that we emit an in-progress event if this is
+ // the first event for the goroutine in the trace
+ // or trace generation. Also, do this between
+ // acquire/release because this is part of the
+ // goroutine's trace state, and it must be atomic
+ // with respect to the tracer.
+ gp.inMarkAssist = false
traceRelease(trace)
+ } else {
+ // This state is tracked even if tracing isn't enabled.
+ // It's only used by the new tracer.
+ // See the comment on enteredMarkAssistForTracing.
+ gp.inMarkAssist = false
}
}
return
if scanWork == 0 {
// We were able to steal all of the credit we
// needed.
- if traced {
+ if enteredMarkAssistForTracing {
trace := traceAcquire()
if trace.ok() {
trace.GCMarkAssistDone()
+ // Set this *after* we trace the end to make sure
+ // that we emit an in-progress event if this is
+ // the first event for the goroutine in the trace
+ // or trace generation. Also, do this between
+ // acquire/release because this is part of the
+ // goroutine's trace state, and it must be atomic
+ // with respect to the tracer.
+ gp.inMarkAssist = false
traceRelease(trace)
+ } else {
+ // This state is tracked even if tracing isn't enabled.
+ // It's only used by the new tracer.
+ // See the comment on enteredMarkAssistForTracing.
+ gp.inMarkAssist = false
}
}
return
}
}
- if traceEnabled() && !traced {
+ if !enteredMarkAssistForTracing {
trace := traceAcquire()
if trace.ok() {
- traced = true
+ if !goexperiment.ExecTracer2 {
+ // In the old tracer, enter mark assist tracing only
+ // if we actually traced an event. Otherwise a goroutine
+ // waking up from mark assist post-GC might end up
+ // writing a stray "end" event.
+ //
+ // This means inMarkAssist will not be meaningful
+ // in the old tracer; that's OK, it's unused.
+ //
+ // See the comment on enteredMarkAssistForTracing.
+ enteredMarkAssistForTracing = true
+ }
trace.GCMarkAssistStart()
+ // Set this *after* we trace the start, otherwise we may
+ // emit an in-progress event for an assist we're about to start.
+ gp.inMarkAssist = true
traceRelease(trace)
+ } else {
+ gp.inMarkAssist = true
+ }
+ if goexperiment.ExecTracer2 {
+ // In the new tracer, set enter mark assist tracing if we
+ // ever pass this point, because we must manage inMarkAssist
+ // correctly.
+ //
+ // See the comment on enteredMarkAssistForTracing.
+ enteredMarkAssistForTracing = true
}
}
// At this point either background GC has satisfied
// this G's assist debt, or the GC cycle is over.
}
- if traced {
+ if enteredMarkAssistForTracing {
trace := traceAcquire()
if trace.ok() {
trace.GCMarkAssistDone()
+ // Set this *after* we trace the end to make sure
+ // that we emit an in-progress event if this is
+ // the first event for the goroutine in the trace
+ // or trace generation. Also, do this between
+ // acquire/release because this is part of the
+ // goroutine's trace state, and it must be atomic
+ // with respect to the tracer.
+ gp.inMarkAssist = false
traceRelease(trace)
+ } else {
+ // This state is tracked even if tracing isn't enabled.
+ // It's only used by the new tracer.
+ // See the comment on enteredMarkAssistForTracing.
+ gp.inMarkAssist = false
}
}
}
// credit to gcController.bgScanCredit every gcCreditSlack units of
// scan work.
//
-// gcDrain will always return if there is a pending STW.
+// gcDrain will always return if there is a pending STW or forEachP.
//
// Disabling write barriers is necessary to ensure that after we've
// confirmed that we've drained gcw, that we don't accidentally end
throw("gcDrain phase incorrect")
}
+ // N.B. We must be running in a non-preemptible context, so it's
+ // safe to hold a reference to our P here.
gp := getg().m.curg
+ pp := gp.m.p.ptr()
preemptible := flags&gcDrainUntilPreempt != 0
flushBgCredit := flags&gcDrainFlushBgCredit != 0
idle := flags&gcDrainIdle != 0
// Drain root marking jobs.
if work.markrootNext < work.markrootJobs {
- // Stop if we're preemptible or if someone wants to STW.
- for !(gp.preempt && (preemptible || sched.gcwaiting.Load())) {
+ // Stop if we're preemptible, if someone wants to STW, or if
+ // someone is calling forEachP.
+ for !(gp.preempt && (preemptible || sched.gcwaiting.Load() || pp.runSafePointFn != 0)) {
job := atomic.Xadd(&work.markrootNext, +1) - 1
if job >= work.markrootJobs {
break
}
// Drain heap marking jobs.
- // Stop if we're preemptible or if someone wants to STW.
- for !(gp.preempt && (preemptible || sched.gcwaiting.Load())) {
+ //
+ // Stop if we're preemptible, if someone wants to STW, or if
+ // someone is calling forEachP.
+ //
+ // TODO(mknyszek): Consider always checking gp.preempt instead
+ // of having the preempt flag, and making an exception for certain
+ // mark workers in retake. That might be simpler than trying to
+ // enumerate all the reasons why we might want to preempt, even
+ // if we're supposed to be mostly non-preemptible.
+ for !(gp.preempt && (preemptible || sched.gcwaiting.Load() || pp.runSafePointFn != 0)) {
// Try to keep work available on the global queue. We used to
// check if there were waiting workers, but it's better to
// just keep work available than to make workers wait. In the