From 8a5af7910a9b157c02736c3e0998a587bb8511c1 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 22 Nov 2019 21:06:15 +0000 Subject: [PATCH] runtime: ready scavenger without next This change makes it so that waking up the scavenger readies its goroutine without "next" set, so that it doesn't interfere with the application's use of the runnext feature in the scheduler which helps fairness. As of CL 201763 the scavenger began waking up much more often, and in TestPingPongHog this meant that it would sometimes supercede either a hog or light goroutine in runnext, leading to a skew in the results and ultimately a test flake. This change thus re-enables the TestPingPongHog test on the builders. Fixes #35271. Change-Id: Iace08576912e8940554dd7de6447e458ad0d201d Reviewed-on: https://go-review.googlesource.com/c/go/+/208380 Run-TryBot: Michael Knyszek TryBot-Result: Gobot Gobot Reviewed-by: Austin Clements --- src/runtime/mgcscavenge.go | 10 ++++++++-- src/runtime/proc_test.go | 2 -- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 9c45ce8c87..c7bab59fb7 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -166,9 +166,15 @@ func wakeScavenger() { stopTimer(scavenge.timer) // Unpark the goroutine and tell it that there may have been a pacing - // change. + // change. Note that we skip the scheduler's runnext slot because we + // want to avoid having the scavenger interfere with the fair + // scheduling of user goroutines. In effect, this schedules the + // scavenger at a "lower priority" but that's OK because it'll + // catch up on the work it missed when it does get scheduled. scavenge.parked = false - goready(scavenge.g, 0) + systemstack(func() { + ready(scavenge.g, 0, false) + }) } unlock(&scavenge.lock) } diff --git a/src/runtime/proc_test.go b/src/runtime/proc_test.go index 48b865e8a5..acee7a1819 100644 --- a/src/runtime/proc_test.go +++ b/src/runtime/proc_test.go @@ -6,7 +6,6 @@ package runtime_test import ( "fmt" - "internal/testenv" "math" "net" "runtime" @@ -423,7 +422,6 @@ func TestPingPongHog(t *testing.T) { if testing.Short() { t.Skip("skipping in -short mode") } - testenv.SkipFlaky(t, 35271) defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(1)) done := make(chan bool) -- 2.50.0