Commit d35dfd40 authored by Austin Clements's avatar Austin Clements

runtime: make assists perform root jobs

Currently, assists can only perform heap marking jobs. However, at the
beginning of GC, there are only root jobs and no heap marking jobs. As
a result, there's often a period at the beginning of a GC cycle where
no goroutine has accumulated assist credit, but at the same time it
can't get any credit because there are no heap marking jobs for it to
do yet. As a result, many goroutines often block on the assist queue
at the very beginning of the GC cycle.

This commit fixes this by allowing assists to perform root marking
jobs. The tricky part of this (and the reason we haven't done this
before) is that stack scanning jobs can lead to deadlocks if the
goroutines performing the stack scanning are themselves
non-preemptible, since two non-preemptible goroutines may try to scan
each other. To address this, we use the same insight d6625caf used to
simplify the mark worker stack scanning: as long as we're careful with
the stacks and only drain jobs while on the system stack, we can put
the goroutine into a preemptible state while we drain jobs. This means
an assist's user stack can be scanned while it continues to do work.

This reduces the rate of assist blocking in the x/benchmarks HTTP
benchmark by a factor of 3 and all remaining blocking happens towards
the *end* of the GC cycle, when there may genuinely not be enough work
to go around.

Ideally, assists would get credit for working on root jobs. Currently
they do not; however, this change prioritizes heap work over root jobs
in assists, so they're likely to mostly perform heap work. In contrast
with mark workers, for assists, the root jobs act only as a backstop
to create heap work when there isn't enough heap work.

Fixes #15361.

Change-Id: If6e169863e4ad75710b0c8dc00f6125b41e9a595
Reviewed-on: https://go-review.googlesource.com/32432Reviewed-by: default avatarRick Hudson <rlh@golang.org>
parent f9e1adb7
......@@ -521,6 +521,10 @@ func gcAssistAlloc1(gp *g, scanWork int64) {
throw("nwait > work.nprocs")
}
// gcDrainN requires the caller to be preemptible.
casgstatus(gp, _Grunning, _Gwaiting)
gp.waitreason = "GC assist marking"
// drain own cached work first in the hopes that it
// will be more cache friendly.
gcw := &getg().m.p.ptr().gcw
......@@ -531,6 +535,8 @@ func gcAssistAlloc1(gp *g, scanWork int64) {
gcw.dispose()
}
casgstatus(gp, _Gwaiting, _Grunning)
// Record that we did this much scan work.
//
// Back out the number of bytes of assist credit that
......@@ -1083,7 +1089,13 @@ func gcDrain(gcw *gcWork, flags gcDrainFlags) {
// buffer. Otherwise, it will perform at least n units of work, but
// may perform more because scanning is always done in whole object
// increments. It returns the amount of scan work performed.
//
// The caller goroutine must be in a preemptible state (e.g.,
// _Gwaiting) to prevent deadlocks during stack scanning. As a
// consequence, this must be called on the system stack.
//
//go:nowritebarrier
//go:systemstack
func gcDrainN(gcw *gcWork, scanWork int64) int64 {
if !writeBarrier.needed {
throw("gcDrainN phase incorrect")
......@@ -1111,6 +1123,18 @@ func gcDrainN(gcw *gcWork, scanWork int64) int64 {
}
if b == 0 {
// Try to do a root job.
//
// TODO: Assists should get credit for this
// work.
if work.markrootNext < work.markrootJobs {
job := atomic.Xadd(&work.markrootNext, +1) - 1
if job < work.markrootJobs {
markroot(gcw, job)
continue
}
}
// No heap or root jobs.
break
}
scanobject(b, gcw)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment