]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: reduce lock/scheduler contention
authorDaniel Morsing <daniel.morsing@gmail.com>
Mon, 1 Dec 2025 09:40:16 +0000 (09:40 +0000)
committerGopher Robot <gobot@golang.org>
Mon, 26 Jan 2026 16:07:51 +0000 (08:07 -0800)
During the parallel section of compilation, we limit the amount of
parallelism to prevent scheduler churn. We do this with a worker
scheduler, but it does not have insight on when a compilation is blocked
due to lock contention.

The symbol table was protected with a lock. While most lookups were
quick lock->read->unlock affairs, sometimes there would be
initialization logic involved. This caused every lookup to stall,
waiting for init. Since our worker scheduler couldn't see this, it would
not launch new goroutine to "cover" the gap.

Fix by splitting the symbol lock into 2 cases, initialization and
lookup. If symbols need initialization simultaneously, they will wait
for each other, but the common case of looking up a symbol will be
handled by a syncmap. In practice, I have yet to see this lock being
blocked on.

Additionally, get rid of the scheduler goroutine and have each
compilation goroutine grab work from a central queue. When multiple
compilations finished at the same time, the work scheduler would
sometime not get run immediately. This ended up starving the system of
work.

These 2 changes together cuts -1.37% off the build time of typescriptgo
on systems with a lot of cores (specifically, the c3h88 perf builder).

Updates #73044.

Change-Id: I6d4b3be56fd00a4fdd4df132bcbd52e4b2a3e91f
Reviewed-on: https://go-review.googlesource.com/c/go/+/724623
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>

src/cmd/compile/internal/gc/compile.go
src/cmd/internal/obj/line_test.go
src/cmd/internal/obj/link.go
src/cmd/internal/obj/sym.go

index 1eb4b8cc37c30c18a8362049d12e0dc4c728b99d..ab9dac2f70da395a15e65fea184da2fe67e81738 100644 (file)
@@ -142,73 +142,47 @@ func compileFunctions(profile *pgoir.Profile) {
                // Compile the longest functions first,
                // since they're most likely to be the slowest.
                // This helps avoid stragglers.
+               // Since we remove from the end of the slice queue,
+               // that means shortest to longest.
                slices.SortFunc(compilequeue, func(a, b *ir.Func) int {
-                       return cmp.Compare(len(b.Body), len(a.Body))
+                       return cmp.Compare(len(a.Body), len(b.Body))
                })
        }
 
-       // By default, we perform work right away on the current goroutine
-       // as the solo worker.
-       queue := func(work func(int)) {
-               work(0)
-       }
+       var mu sync.Mutex
+       var wg sync.WaitGroup
+       mu.Lock()
 
-       if nWorkers := base.Flag.LowerC; nWorkers > 1 {
-               // For concurrent builds, we allow the work queue
-               // to grow arbitrarily large, but only nWorkers work items
-               // can be running concurrently.
-               workq := make(chan func(int))
-               done := make(chan int)
+       for workerId := range base.Flag.LowerC {
+               // TODO: replace with wg.Go when the oldest bootstrap has it.
+               // With the current policy, that'd be go1.27.
+               wg.Add(1)
                go func() {
-                       ids := make([]int, nWorkers)
-                       for i := range ids {
-                               ids[i] = i
-                       }
-                       var pending []func(int)
+                       defer wg.Done()
+                       var closures []*ir.Func
                        for {
-                               select {
-                               case work := <-workq:
-                                       pending = append(pending, work)
-                               case id := <-done:
-                                       ids = append(ids, id)
-                               }
-                               for len(pending) > 0 && len(ids) > 0 {
-                                       work := pending[len(pending)-1]
-                                       id := ids[len(ids)-1]
-                                       pending = pending[:len(pending)-1]
-                                       ids = ids[:len(ids)-1]
-                                       go func() {
-                                               work(id)
-                                               done <- id
-                                       }()
+                               mu.Lock()
+                               compilequeue = append(compilequeue, closures...)
+                               remaining := len(compilequeue)
+                               if remaining == 0 {
+                                       mu.Unlock()
+                                       return
                                }
+                               fn := compilequeue[len(compilequeue)-1]
+                               compilequeue = compilequeue[:len(compilequeue)-1]
+                               mu.Unlock()
+                               ssagen.Compile(fn, workerId, profile)
+                               closures = fn.Closures
                        }
                }()
-               queue = func(work func(int)) {
-                       workq <- work
-               }
-       }
-
-       var wg sync.WaitGroup
-       var compile func([]*ir.Func)
-       compile = func(fns []*ir.Func) {
-               wg.Add(len(fns))
-               for _, fn := range fns {
-                       fn := fn
-                       queue(func(worker int) {
-                               ssagen.Compile(fn, worker, profile)
-                               compile(fn.Closures)
-                               wg.Done()
-                       })
-               }
        }
 
        types.CalcSizeDisabled = true // not safe to calculate sizes concurrently
        base.Ctxt.InParallel = true
 
-       compile(compilequeue)
-       compilequeue = nil
+       mu.Unlock()
        wg.Wait()
+       compilequeue = nil
 
        base.Ctxt.InParallel = false
        types.CalcSizeDisabled = false
index de7ef1a22eb1c97645fe46f49d9866e83a7d09ea..2cf3d3088b1bd58eb1fda2791d62b27eec999cb6 100644 (file)
@@ -12,7 +12,6 @@ import (
 
 func TestGetFileSymbolAndLine(t *testing.T) {
        ctxt := new(Link)
-       ctxt.hash = make(map[string]*LSym)
        ctxt.statichash = make(map[string]*LSym)
 
        afile := src.NewFileBase("a.go", "a.go")
index c70c1d94384545ec2917d504852adb0fa2177de7..05ffba11ea1ad04a9d43510dbf1ad8fc57394f13 100644 (file)
@@ -1169,21 +1169,22 @@ type Link struct {
        Flag_maymorestack    string // If not "", call this function before stack checks
        Bso                  *bufio.Writer
        Pathname             string
-       Pkgpath              string           // the current package's import path
-       hashmu               sync.Mutex       // protects hash, funchash
-       hash                 map[string]*LSym // name -> sym mapping
-       funchash             map[string]*LSym // name -> sym mapping for ABIInternal syms
-       statichash           map[string]*LSym // name -> sym mapping for static syms
-       PosTable             src.PosTable
-       InlTree              InlTree // global inlining tree used by gc/inl.go
-       DwFixups             *DwarfFixupTable
-       DwTextCount          int
-       Imports              []goobj.ImportedPkg
-       DiagFunc             func(string, ...any)
-       DiagFlush            func()
-       DebugInfo            func(ctxt *Link, fn *LSym, info *LSym, curfn Func) ([]dwarf.Scope, dwarf.InlCalls)
-       GenAbstractFunc      func(fn *LSym)
-       Errors               int
+       Pkgpath              string // the current package's import path
+
+       hashmu          sync.Mutex       // protects hash, funchash
+       hash            sync.Map         // name(string) -> sym(*syncOnce) mapping
+       funchash        sync.Map         // name(string) -> sym(*syncOnce) mapping for ABIInternal syms
+       statichash      map[string]*LSym // name -> sym mapping for static syms
+       PosTable        src.PosTable
+       InlTree         InlTree // global inlining tree used by gc/inl.go
+       DwFixups        *DwarfFixupTable
+       DwTextCount     int
+       Imports         []goobj.ImportedPkg
+       DiagFunc        func(string, ...any)
+       DiagFlush       func()
+       DebugInfo       func(ctxt *Link, fn *LSym, info *LSym, curfn Func) ([]dwarf.Scope, dwarf.InlCalls)
+       GenAbstractFunc func(fn *LSym)
+       Errors          int
 
        InParallel    bool // parallel backend phase in effect
        UseBASEntries bool // use Base Address Selection Entries in location lists and PC ranges
@@ -1217,6 +1218,13 @@ type Link struct {
        Fingerprint goobj.FingerprintType // fingerprint of symbol indices, to catch index mismatch
 }
 
+// symOnce is a "marker" value for our sync.Maps. We insert it on lookup and
+// use the atomic value to check whether initialization has been completed.
+type symOnce struct {
+       sym    LSym
+       inited atomic.Bool
+}
+
 // Assert to vet's printf checker that Link.DiagFunc is a printf-like.
 func _(ctxt *Link) {
        ctxt.DiagFunc = func(format string, args ...any) {
index 08c50ec72b6a5c49222b72cfb8e8f966d2c3a2a0..acd9d9cf6039ff2c2517b21d14ca4cca85d351f0 100644 (file)
@@ -42,12 +42,11 @@ import (
        "log"
        "math"
        "sort"
+       "sync"
 )
 
 func Linknew(arch *LinkArch) *Link {
        ctxt := new(Link)
-       ctxt.hash = make(map[string]*LSym)
-       ctxt.funchash = make(map[string]*LSym)
        ctxt.statichash = make(map[string]*LSym)
        ctxt.Arch = arch
        ctxt.Pathname = objabi.WorkingDir()
@@ -90,28 +89,34 @@ func (ctxt *Link) LookupABI(name string, abi ABI) *LSym {
 // If it does not exist, it creates it and
 // passes it to init for one-time initialization.
 func (ctxt *Link) LookupABIInit(name string, abi ABI, init func(s *LSym)) *LSym {
-       var hash map[string]*LSym
+       var hash *sync.Map
        switch abi {
        case ABI0:
-               hash = ctxt.hash
+               hash = &ctxt.hash
        case ABIInternal:
-               hash = ctxt.funchash
+               hash = &ctxt.funchash
        default:
                panic("unknown ABI")
        }
 
-       ctxt.hashmu.Lock()
-       s := hash[name]
-       if s == nil {
-               s = &LSym{Name: name}
-               s.SetABI(abi)
-               hash[name] = s
-               if init != nil {
-                       init(s)
+       c, _ := hash.Load(name)
+       if c == nil {
+               once := &symOnce{
+                       sym: LSym{Name: name},
                }
+               once.sym.SetABI(abi)
+               c, _ = hash.LoadOrStore(name, once)
        }
-       ctxt.hashmu.Unlock()
-       return s
+       once := c.(*symOnce)
+       if init != nil && !once.inited.Load() {
+               ctxt.hashmu.Lock()
+               if !once.inited.Load() {
+                       init(&once.sym)
+                       once.inited.Store(true)
+               }
+               ctxt.hashmu.Unlock()
+       }
+       return &once.sym
 }
 
 // Lookup looks up the symbol with name name.
@@ -124,17 +129,31 @@ func (ctxt *Link) Lookup(name string) *LSym {
 // If it does not exist, it creates it and
 // passes it to init for one-time initialization.
 func (ctxt *Link) LookupInit(name string, init func(s *LSym)) *LSym {
-       ctxt.hashmu.Lock()
-       s := ctxt.hash[name]
-       if s == nil {
-               s = &LSym{Name: name}
-               ctxt.hash[name] = s
-               if init != nil {
-                       init(s)
+       c, _ := ctxt.hash.Load(name)
+       if c == nil {
+               once := &symOnce{
+                       sym: LSym{Name: name},
                }
+               c, _ = ctxt.hash.LoadOrStore(name, once)
        }
-       ctxt.hashmu.Unlock()
-       return s
+       once := c.(*symOnce)
+       if init != nil && !once.inited.Load() {
+               // TODO(dmo): some of our init functions modify other fields
+               // in the symbol table. They are only implicitly protected since
+               // we serialize all inits under the hashmu lock.
+               // Consider auditing the functions and have them lock their
+               // concurrent access values explicitly. This would make it possible
+               // to have more than one than one init going at a time (although this might
+               // be a theoretical concern, I have yet to catch this lock actually being waited
+               // on).
+               ctxt.hashmu.Lock()
+               if !once.inited.Load() {
+                       init(&once.sym)
+                       once.inited.Store(true)
+               }
+               ctxt.hashmu.Unlock()
+       }
+       return &once.sym
 }
 
 func (ctxt *Link) rodataKind() (suffix string, typ objabi.SymKind) {