diff options
author | Michael Anthony Knyszek <mknyszek@google.com> | 2023-03-27 15:47:18 +0000 |
---|---|---|
committer | Michael Knyszek <mknyszek@google.com> | 2023-07-21 00:28:09 +0000 |
commit | 4f4c23512e2d5cddfa7024a7e9c217ba357b9978 (patch) | |
tree | 82a945eacc4a4bee1e7dc7f73a7c7072e5be4e03 /src/cmd/dist/test.go | |
parent | 16ec51471288f86d601efb9a2eb6b418cf634c16 (diff) | |
download | go-4f4c23512e2d5cddfa7024a7e9c217ba357b9978.tar.gz go-4f4c23512e2d5cddfa7024a7e9c217ba357b9978.zip |
runtime: fix debug non-concurrent sweep mode after activeSweep changes
Currently the GC creates a sweepLocker before restarting the world at
the end of the mark phase, so that it can safely flush mcaches without
the runtime incorrectly concluding that sweeping is done before that
happens.
However, with GODEBUG=gcstoptheworld=2, where sweeping happens during
that STW phase, creating that sweepLocker will fail, since the runtime
will conclude that sweeping is in fact complete (all the queues will be
drained). The problem however is that gcSweep, which does the
non-concurrent sweeping, doesn't actually flush mcaches.
In essence, this failure to create a sweepLocker is indicating a real
issue: sweeping is marked as complete, but we haven't flush the mcaches
yet!
The fix to this is to flush mcaches in gcSweep when in a non-concurrent
sweep. Now that gcSweep actually completes a full sweep, it's safe to
ignore a failure to create a sweepLocker (and in fact, it *must* fail).
While we're here, let's also remove _ConcurrentSweep, the debug flag.
There's already an alias for it called concurrentSweep, and there's only
one use of it in gcSweep.
Lastly, add a dist test for the GODEBUG=gcstoptheworld=2 mode.
Fixes #53885.
Change-Id: I8a1e5b8f362ed8abd03f76e4950d3211f145ab1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/479517
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Diffstat (limited to 'src/cmd/dist/test.go')
-rw-r--r-- | src/cmd/dist/test.go | 18 |
1 files changed, 16 insertions, 2 deletions
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index f148fb97b5..bd02149c11 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -676,8 +676,22 @@ func (t *tester) registerTests() { }) } - // morestack tests. We only run these on in long-test mode - // (with GO_TEST_SHORT=false) because the runtime test is + // GODEBUG=gcstoptheworld=2 tests. We only run these in long-test + // mode (with GO_TEST_SHORT=0) because this is just testing a + // non-critical debug setting. + if !t.compileOnly && !t.short { + t.registerTest("GODEBUG=gcstoptheworld=2 archive/zip", + &goTest{ + variant: "runtime:gcstoptheworld2", + timeout: 300 * time.Second, + short: true, + env: []string{"GODEBUG=gcstoptheworld=2"}, + pkg: "archive/zip", + }) + } + + // morestack tests. We only run these in long-test mode + // (with GO_TEST_SHORT=0) because the runtime test is // already quite long and mayMoreStackMove makes it about // twice as slow. if !t.compileOnly && !t.short { |