Skip to content

Commit

Permalink
runtime: make p.gcBgMarkWorker a guintptr
Browse files Browse the repository at this point in the history
Currently p.gcBgMarkWorker is a *g. Change it to a guintptr. This
eliminates a write barrier during the subtle mark worker parking dance
(which isn't known to be causing problems, but may).

Change-Id: Ibf12c05ac910820448059e69a68e5b882c993ed8
Reviewed-on: https://go-review.googlesource.com/18970
Run-TryBot: Austin Clements <[email protected]>
Reviewed-by: Rick Hudson <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
  • Loading branch information
aclements authored and rsc committed Jan 27, 2016
1 parent 08594ac commit 09940b9
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 19 deletions.
17 changes: 6 additions & 11 deletions src/runtime/mgc.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ func (c *gcControllerState) findRunnableGCWorker(_p_ *p) *g {
if gcBlackenEnabled == 0 {
throw("gcControllerState.findRunnable: blackening not enabled")
}
if _p_.gcBgMarkWorker == nil {
if _p_.gcBgMarkWorker == 0 {
// The mark worker associated with this P is blocked
// performing a mark transition. We can't run it
// because it may be on some other run or wait queue.
Expand Down Expand Up @@ -711,7 +711,7 @@ func (c *gcControllerState) findRunnableGCWorker(_p_ *p) *g {
}

// Run the background mark worker
gp := _p_.gcBgMarkWorker
gp := _p_.gcBgMarkWorker.ptr()
casgstatus(gp, _Gwaiting, _Grunnable)
if trace.enabled {
traceGoUnpark(gp, 0)
Expand Down Expand Up @@ -1325,7 +1325,7 @@ func gcBgMarkStartWorkers() {
if p == nil || p.status == _Pdead {
break
}
if p.gcBgMarkWorker == nil {
if p.gcBgMarkWorker == 0 {
go gcBgMarkWorker(p)
notetsleepg(&work.bgMarkReady, -1)
noteclear(&work.bgMarkReady)
Expand Down Expand Up @@ -1356,11 +1356,6 @@ func gcBgMarkWorker(_p_ *p) {
}
var park parkInfo

// casgp is casp for *g's.
casgp := func(gpp **g, old, new *g) bool {
return casp((*unsafe.Pointer)(unsafe.Pointer(gpp)), unsafe.Pointer(old), unsafe.Pointer(new))
}

gp := getg()
park.m = acquirem()
park.attach = _p_
Expand Down Expand Up @@ -1397,7 +1392,7 @@ func gcBgMarkWorker(_p_ *p) {
// cas the worker because we may be
// racing with a new worker starting
// on this P.
if !casgp(&p.gcBgMarkWorker, nil, g) {
if !p.gcBgMarkWorker.cas(0, guintptr(unsafe.Pointer(g))) {
// The P got a new worker.
// Exit this worker.
return false
Expand All @@ -1409,7 +1404,7 @@ func gcBgMarkWorker(_p_ *p) {
// Loop until the P dies and disassociates this
// worker (the P may later be reused, in which case
// it will get a new worker) or we failed to associate.
if _p_.gcBgMarkWorker != gp {
if _p_.gcBgMarkWorker.ptr() != gp {
break
}

Expand Down Expand Up @@ -1478,7 +1473,7 @@ func gcBgMarkWorker(_p_ *p) {
// as the worker for this P so
// findRunnableGCWorker doesn't try to
// schedule it.
_p_.gcBgMarkWorker = nil
_p_.gcBgMarkWorker.set(nil)
releasem(park.m)

gcMarkDone()
Expand Down
14 changes: 7 additions & 7 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1867,9 +1867,9 @@ stop:
// We have nothing to do. If we're in the GC mark phase, can
// safely scan and blacken objects, and have work to do, run
// idle-time marking rather than give up the P.
if _p_ := _g_.m.p.ptr(); gcBlackenEnabled != 0 && _p_.gcBgMarkWorker != nil && gcMarkWorkAvailable(_p_) {
if _p_ := _g_.m.p.ptr(); gcBlackenEnabled != 0 && _p_.gcBgMarkWorker != 0 && gcMarkWorkAvailable(_p_) {
_p_.gcMarkWorkerMode = gcMarkWorkerIdleMode
gp := _p_.gcBgMarkWorker
gp := _p_.gcBgMarkWorker.ptr()
casgstatus(gp, _Gwaiting, _Grunnable)
if trace.enabled {
traceGoUnpark(gp, 0)
Expand Down Expand Up @@ -3206,15 +3206,15 @@ func procresize(nprocs int32) *p {
}
// if there's a background worker, make it runnable and put
// it on the global queue so it can clean itself up
if p.gcBgMarkWorker != nil {
casgstatus(p.gcBgMarkWorker, _Gwaiting, _Grunnable)
if gp := p.gcBgMarkWorker.ptr(); gp != nil {
casgstatus(gp, _Gwaiting, _Grunnable)
if trace.enabled {
traceGoUnpark(p.gcBgMarkWorker, 0)
traceGoUnpark(gp, 0)
}
globrunqput(p.gcBgMarkWorker)
globrunqput(gp)
// This assignment doesn't race because the
// world is stopped.
p.gcBgMarkWorker = nil
p.gcBgMarkWorker.set(nil)
}
for i := range p.sudogbuf {
p.sudogbuf[i] = nil
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/runtime2.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ type p struct {

// Per-P GC state
gcAssistTime int64 // Nanoseconds in assistAlloc
gcBgMarkWorker *g
gcBgMarkWorker guintptr
gcMarkWorkerMode gcMarkWorkerMode

// gcw is this P's GC work buffer cache. The work buffer is
Expand Down

0 comments on commit 09940b9

Please sign in to comment.