Skip to content

Commit e761556

Browse files
committed
runtime: convert gcController.dedicatedMarkWorkersNeeded to atomic type
In gcController.startCycle we just compute the initial value in a local variable before assigning to the atomic field to avoid noisy churn. For #53821. Change-Id: Ibde0ac8fd49aa6bbee3bd02fe3ffb17429abd5a9 Reviewed-on: https://go-review.googlesource.com/c/go/+/417784 Run-TryBot: Michael Pratt <[email protected]> Reviewed-by: Austin Clements <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent dcd1037 commit e761556

File tree

2 files changed

+20
-25
lines changed

2 files changed

+20
-25
lines changed

src/runtime/align_runtime_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ var AtomicFields = []uintptr{
2121
unsafe.Offsetof(schedt{}.lastpoll),
2222
unsafe.Offsetof(schedt{}.pollUntil),
2323
unsafe.Offsetof(schedt{}.timeToRun),
24-
unsafe.Offsetof(gcControllerState{}.dedicatedMarkWorkersNeeded),
2524
unsafe.Offsetof(timeHistogram{}.underflow),
2625
unsafe.Offsetof(profBuf{}.overflow),
2726
unsafe.Offsetof(profBuf{}.overflowTime),

src/runtime/mgcpacer.go

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ type gcControllerState struct {
9292
// Initialized from GOGC. GOGC=off means no GC.
9393
gcPercent atomic.Int32
9494

95-
_ uint32 // padding so following 64-bit values are 8-byte aligned
96-
9795
// memoryLimit is the soft memory limit in bytes.
9896
//
9997
// Initialized from GOMEMLIMIT. GOMEMLIMIT=off is equivalent to MaxInt64
@@ -145,8 +143,6 @@ type gcControllerState struct {
145143
// consMark; see consMark for details.
146144
consMarkController piController
147145

148-
_ uint32 // Padding for atomics on 32-bit platforms.
149-
150146
// gcPercentHeapGoal is the goal heapLive for when next GC ends derived
151147
// from gcPercent.
152148
//
@@ -289,11 +285,10 @@ type gcControllerState struct {
289285
// that assists and background mark workers started.
290286
markStartTime int64
291287

292-
// dedicatedMarkWorkersNeeded is the number of dedicated mark
293-
// workers that need to be started. This is computed at the
294-
// beginning of each cycle and decremented atomically as
295-
// dedicated mark workers get started.
296-
dedicatedMarkWorkersNeeded int64
288+
// dedicatedMarkWorkersNeeded is the number of dedicated mark workers
289+
// that need to be started. This is computed at the beginning of each
290+
// cycle and decremented as dedicated mark workers get started.
291+
dedicatedMarkWorkersNeeded atomic.Int64
297292

298293
// idleMarkWorkers is two packed int32 values in a single uint64.
299294
// These two values are always updated simultaneously.
@@ -448,26 +443,26 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
448443
// 25%. For small GOMAXPROCS, this would introduce too much
449444
// error, so we add fractional workers in that case.
450445
totalUtilizationGoal := float64(procs) * gcBackgroundUtilization
451-
c.dedicatedMarkWorkersNeeded = int64(totalUtilizationGoal + 0.5)
452-
utilError := float64(c.dedicatedMarkWorkersNeeded)/totalUtilizationGoal - 1
446+
dedicatedMarkWorkersNeeded := int64(totalUtilizationGoal + 0.5)
447+
utilError := float64(dedicatedMarkWorkersNeeded)/totalUtilizationGoal - 1
453448
const maxUtilError = 0.3
454449
if utilError < -maxUtilError || utilError > maxUtilError {
455450
// Rounding put us more than 30% off our goal. With
456451
// gcBackgroundUtilization of 25%, this happens for
457452
// GOMAXPROCS<=3 or GOMAXPROCS=6. Enable fractional
458453
// workers to compensate.
459-
if float64(c.dedicatedMarkWorkersNeeded) > totalUtilizationGoal {
454+
if float64(dedicatedMarkWorkersNeeded) > totalUtilizationGoal {
460455
// Too many dedicated workers.
461-
c.dedicatedMarkWorkersNeeded--
456+
dedicatedMarkWorkersNeeded--
462457
}
463-
c.fractionalUtilizationGoal = (totalUtilizationGoal - float64(c.dedicatedMarkWorkersNeeded)) / float64(procs)
458+
c.fractionalUtilizationGoal = (totalUtilizationGoal - float64(dedicatedMarkWorkersNeeded)) / float64(procs)
464459
} else {
465460
c.fractionalUtilizationGoal = 0
466461
}
467462

468463
// In STW mode, we just want dedicated workers.
469464
if debug.gcstoptheworld > 0 {
470-
c.dedicatedMarkWorkersNeeded = int64(procs)
465+
dedicatedMarkWorkersNeeded = int64(procs)
471466
c.fractionalUtilizationGoal = 0
472467
}
473468

@@ -482,7 +477,7 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
482477
// required. However, we need at least one dedicated mark worker or
483478
// idle GC worker to ensure GC progress in some scenarios (see comment
484479
// on maxIdleMarkWorkers).
485-
if c.dedicatedMarkWorkersNeeded > 0 {
480+
if dedicatedMarkWorkersNeeded > 0 {
486481
c.setMaxIdleMarkWorkers(0)
487482
} else {
488483
// TODO(mknyszek): The fundamental reason why we need this is because
@@ -492,13 +487,14 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
492487
c.setMaxIdleMarkWorkers(1)
493488
}
494489
} else {
495-
// N.B. gomaxprocs and dedicatedMarkWorkersNeeded is guaranteed not to
490+
// N.B. gomaxprocs and dedicatedMarkWorkersNeeded are guaranteed not to
496491
// change during a GC cycle.
497-
c.setMaxIdleMarkWorkers(int32(procs) - int32(c.dedicatedMarkWorkersNeeded))
492+
c.setMaxIdleMarkWorkers(int32(procs) - int32(dedicatedMarkWorkersNeeded))
498493
}
499494

500495
// Compute initial values for controls that are updated
501496
// throughout the cycle.
497+
c.dedicatedMarkWorkersNeeded.Store(dedicatedMarkWorkersNeeded)
502498
c.revise()
503499

504500
if debug.gcpacertrace > 0 {
@@ -507,7 +503,7 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
507503
" (scan ", gcController.heapScan.Load()>>20, " MB in ",
508504
work.initialHeapLive>>20, "->",
509505
heapGoal>>20, " MB)",
510-
" workers=", c.dedicatedMarkWorkersNeeded,
506+
" workers=", dedicatedMarkWorkersNeeded,
511507
"+", c.fractionalUtilizationGoal, "\n")
512508
}
513509
}
@@ -761,7 +757,7 @@ func (c *gcControllerState) enlistWorker() {
761757

762758
// There are no idle Ps. If we need more dedicated workers,
763759
// try to preempt a running P so it will switch to a worker.
764-
if c.dedicatedMarkWorkersNeeded <= 0 {
760+
if c.dedicatedMarkWorkersNeeded.Load() <= 0 {
765761
return
766762
}
767763
// Pick a random other P to preempt.
@@ -831,14 +827,14 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
831827
return nil, now
832828
}
833829

834-
decIfPositive := func(ptr *int64) bool {
830+
decIfPositive := func(val *atomic.Int64) bool {
835831
for {
836-
v := atomic.Loadint64(ptr)
832+
v := val.Load()
837833
if v <= 0 {
838834
return false
839835
}
840836

841-
if atomic.Casint64(ptr, v, v-1) {
837+
if val.CompareAndSwap(v, v-1) {
842838
return true
843839
}
844840
}
@@ -905,7 +901,7 @@ func (c *gcControllerState) markWorkerStop(mode gcMarkWorkerMode, duration int64
905901
switch mode {
906902
case gcMarkWorkerDedicatedMode:
907903
c.dedicatedMarkTime.Add(duration)
908-
atomic.Xaddint64(&c.dedicatedMarkWorkersNeeded, 1)
904+
c.dedicatedMarkWorkersNeeded.Add(1)
909905
case gcMarkWorkerFractionalMode:
910906
c.fractionalMarkTime.Add(duration)
911907
case gcMarkWorkerIdleMode:

0 commit comments

Comments
 (0)