Having the goroutines attempting to help out early in the GC while roots
are still being scanned seems like the problem. Fighting for a small amount
of work at this point might not be efficient. In the case where the root
scan is not complete and there are no work bufs available we could just
allow the goroutine to go ahead and allocate. This is part of the larger
discussion about what kinds of availability promises the Go GC wants to
make. At this point 200us is in the noise, hopefully it won't be in the
noise in the 1.8 timeframe.

On Tue, Apr 19, 2016 at 2:01 AM, Dmitry Vyukov notifications@github.com
wrote:

go version devel +318da8d Sat Apr 16 02:55:03 2016 +0000 linux/amd64

I've looked at http benchmark trace:
[image: trace]
https://cloud.githubusercontent.com/assets/1095328/14628858/f047c932-0601-11e6-8405-ce35de5bb539.png
trace.zip https://github.com/golang/go/files/225283/trace.zip

At the beginning of marking all goroutines are both in debt and cannot
scan anything (because GC goroutines are busy with roots and did not
produce enough workbufs yet). So what happens is that all procs cycle
through all runnable goroutines and park them on assistQueue. That's first
200us of GC with small goroutine boxes all blocking in gcAssistAlloc.

gcAssistAlloc must be able to help with root scanning, because it may be
the only available GC work. Switching to other goroutines makes small
sense, because the first malloc will trigger assist and will bring us to
where we started.

Maybe it also makes sense to give goroutines some temporal credit in the
beginning of GC. Currently we start concurrent phase just to force every
goroutine to assist. If it turns out that we need aggressive assists, we
can make up for it later. But in the expected case GC is triggered at such
point that we don't need any assists. The idea is similar to
https://go-review.googlesource.com/#/c/20969 but not black and white
(GOGC=199 requires assists, while GOGC=200 does not need them entirely).

You can also see that Proc 4 is idle for some reason and does not help GC.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#15361

I agree with @dvyukov that assists should be able to scan roots. I don't see why they shouldn't; the amount of fighting is bounded by the number of Ps whether it's assists doing it or the background mark worker. The difficulty is that we currently don't count root scanning as "scan work", so assists can't get credit for it (and can't satisfy their debt with it). This also means that the background worker won't build up global credit while it's scanning roots, which is probably contributing to the delay. Tracking the stats on that is somewhat subtle, which is why we don't do it right now, but it's definitely doable.

I believe we'll also want this when we start re-scanning stacks during concurrent mark so that assists can help out with that.

In the case where the root scan is not complete and there are no work bufs available we could just allow the goroutine to go ahead and allocate.

We used to do this and then I worked very hard to eliminate this uncontrolled allocation window because @dvyukov had some great benchmarks that used this window to completely blow out the heap goal.

Tracking the stats on that is somewhat subtle, which is why we don't do it right now, but it's definitely doable.

But it should be positive without stats. Threads will do useful work instead of switching goroutines.

We used to do this and then I worked very hard to eliminate this uncontrolled allocation window because @dvyukov had some great benchmarks that used this window to completely blow out the heap goal.

Can't we give them temporal credit? The intention is that background scanners will cover that backlog soon and we can allow goroutines to work without assists at all in the expected case. But if background scanners do not, then goroutines will have to assists aggressively and compensate for initial allocations. The duration of the temporal credit must not be time based, it must be allocation based. That is, if we expect to do scanning while application allocates X bytes, we can give credit for X/10 allocations. I don't know the assist system good enough to say whether such credit system can avoid overallocation.
It also looks like a more reliable solution to the problem described in cl/20969. The change turns off assists when GOGC>200. But that still can lead to overallocation. Instead we could give goroutines credit for longer period of time.

@aclements Is this bug still relevant with the new changes to reduce the amount of extra work that is done for each assist?

@aclements Is this bug still relevant with the new changes to reduce the amount of extra work that is done for each assist?

Yes. This issue arises because there's no work at all for assists at the beginning of the cycle, not because they want more work than they can get. Reducing the extra work does mean assists become useful earlier in the cycle, just not yet at the beginning.

CL https://golang.org/cl/32431 mentions this issue.

CL https://golang.org/cl/32432 mentions this issue.