runtime: refactor the scavenger and make it testable

This change refactors the scavenger into a type whose methods represent
the actual function and scheduling of the scavenger. It also stubs out
access to global state in order to make it testable.

This change thus also adds a test for the scavenger. In writing this
test, I discovered the lack of a behavior I expected: if the
pageAlloc.scavenge returns < the bytes requested scavenged, that means
the heap is exhausted. This has been true this whole time, but was not
documented or explicitly relied upon. This change rectifies that. In
theory this means the scavenger could spin in run() indefinitely (as
happened in the test) if shouldStop never told it to stop. In practice,
shouldStop fires long before the heap is exhausted, but for future
changes it may be important. At the very least it's good to be
intentional about these things.

While we're here, I also moved the call to stopTimer out of wake and
into sleep. There's no reason to add more operations to a context that's
already precarious (running without a P on sysmon).

Change-Id: Ib31b86379fd9df84f25ae282734437afc540da5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/384734
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Michael Anthony Knyszek 2022-02-10 00:49:44 +00:00 committed by Michael Knyszek
parent d8cf2243e0
commit d29f5247b8
5 changed files with 577 additions and 228 deletions

View file

@ -1372,3 +1372,107 @@ func NewPIController(kp, ti, tt, min, max float64) *PIController {
func (c *PIController) Next(input, setpoint, period float64) (float64, bool) {
return c.piController.next(input, setpoint, period)
}
const ScavengePercent = scavengePercent
type Scavenger struct {
Sleep func(int64) int64
Scavenge func(uintptr) (uintptr, int64)
ShouldStop func() bool
GoMaxProcs func() int32
released atomic.Uintptr
scavenger scavengerState
stop chan<- struct{}
done <-chan struct{}
}
func (s *Scavenger) Start() {
if s.Sleep == nil || s.Scavenge == nil || s.ShouldStop == nil || s.GoMaxProcs == nil {
panic("must populate all stubs")
}
// Install hooks.
s.scavenger.sleepStub = s.Sleep
s.scavenger.scavenge = s.Scavenge
s.scavenger.shouldStop = s.ShouldStop
s.scavenger.gomaxprocs = s.GoMaxProcs
// Start up scavenger goroutine, and wait for it to be ready.
stop := make(chan struct{})
s.stop = stop
done := make(chan struct{})
s.done = done
go func() {
// This should match bgscavenge, loosely.
s.scavenger.init()
s.scavenger.park()
for {
select {
case <-stop:
close(done)
return
default:
}
released, workTime := s.scavenger.run()
if released == 0 {
s.scavenger.park()
continue
}
s.released.Add(released)
s.scavenger.sleep(workTime)
}
}()
if !s.BlockUntilParked(1e9 /* 1 second */) {
panic("timed out waiting for scavenger to get ready")
}
}
// BlockUntilParked blocks until the scavenger parks, or until
// timeout is exceeded. Returns true if the scavenger parked.
//
// Note that in testing, parked means something slightly different.
// In anger, the scavenger parks to sleep, too, but in testing,
// it only parks when it actually has no work to do.
func (s *Scavenger) BlockUntilParked(timeout int64) bool {
// Just spin, waiting for it to park.
//
// The actual parking process is racy with respect to
// wakeups, which is fine, but for testing we need something
// a bit more robust.
start := nanotime()
for nanotime()-start < timeout {
lock(&s.scavenger.lock)
parked := s.scavenger.parked
unlock(&s.scavenger.lock)
if parked {
return true
}
Gosched()
}
return false
}
// Released returns how many bytes the scavenger released.
func (s *Scavenger) Released() uintptr {
return s.released.Load()
}
// Wake wakes up a parked scavenger to keep running.
func (s *Scavenger) Wake() {
s.scavenger.wake()
}
// Stop cleans up the scavenger's resources. The scavenger
// must be parked for this to work.
func (s *Scavenger) Stop() {
lock(&s.scavenger.lock)
parked := s.scavenger.parked
unlock(&s.scavenger.lock)
if !parked {
panic("tried to clean up scavenger that is not parked")
}
close(s.stop)
s.Wake()
<-s.done
}