From 93d7d1685ee9e9f296e20f6c712796e54602e891 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 20:17:40 +0000 Subject: [PATCH] runtime: load gcControllerState.scanWork atomically in revise gcControllerState.scanWork's docs state that it must be accessed atomically during a GC cycle, but gcControllerState.revise does not do this (even when called with the heap lock held). This change makes it so that gcControllerState.revise accesses scanWork atomically and explicitly. Note that we don't update gcControllerState.revise's erroneous doc comment here because this change isn't about revise's guarantees, just about heap_scan. The comment is updated in a later change. Change-Id: Iafc3ad214e517190bfd8a219896d23da19f7659d Reviewed-on: https://go-review.googlesource.com/c/go/+/246961 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/mgc.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 94539dd770..4b9a6da3b3 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -495,6 +495,7 @@ func (c *gcControllerState) revise() { } live := atomic.Load64(&memstats.heap_live) scan := atomic.Load64(&memstats.heap_scan) + work := atomic.Loadint64(&c.scanWork) // Assume we're under the soft goal. Pace GC to complete at // next_gc assuming the heap is in steady-state. @@ -511,7 +512,7 @@ func (c *gcControllerState) revise() { // 100*heap_scan.) scanWorkExpected := int64(float64(scan) * 100 / float64(100+gcpercent)) - if live > memstats.next_gc || c.scanWork > scanWorkExpected { + if live > memstats.next_gc || work > scanWorkExpected { // We're past the soft goal, or we've already done more scan // work than we expected. Pace GC so that in the worst case it // will complete by the hard goal. @@ -529,7 +530,7 @@ func (c *gcControllerState) revise() { // (scanWork), so allocation will change this difference // slowly in the soft regime and not at all in the hard // regime. - scanWorkRemaining := scanWorkExpected - c.scanWork + scanWorkRemaining := scanWorkExpected - work if scanWorkRemaining < 1000 { // We set a somewhat arbitrary lower bound on // remaining scan work since if we aim a little high, -- 2.48.1