From d09a8c8ef473ba51244d6da9118b956bedb1b20d Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 21 Oct 2025 15:04:46 -0700 Subject: [PATCH] go/types, types2: simplify locking in Named.resolveUnderlying Avoid calling Named.resolveUnderlying in the first place (there is only one caller) if Named.underlying exists already. In Named.resolveUnderlying remove initial atomic check because of the check in Named.Underlying. Also, remove a 2nd atomic check after acquiring the lock as it likely won't help much. Change-Id: Ife87218fa2549d0903a10218f4dd7a70f85d6c7c Reviewed-on: https://go-review.googlesource.com/c/go/+/713521 Auto-Submit: Robert Griesemer LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Griesemer Reviewed-by: Mark Freeman --- src/cmd/compile/internal/types2/named.go | 20 ++++++++------------ src/go/types/named.go | 20 ++++++++------------ 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index 99e2743484..26be3104b8 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -562,7 +562,10 @@ func (n *Named) Underlying() Type { } } - n.resolveUnderlying() + if !n.stateHas(underlying) { + n.resolveUnderlying() + } + return n.underlying } @@ -588,11 +591,6 @@ func (t *Named) String() string { return TypeString(t, nil) } func (n *Named) resolveUnderlying() { assert(n.stateHas(resolved)) - // optimization for likely case - if n.stateHas(underlying) { - return - } - var seen map[*Named]int // allocated lazily var u Type for rhs := Type(n); u == nil; { @@ -618,12 +616,7 @@ func (n *Named) resolveUnderlying() { break } - // acquire the lock without checking; performance is probably fine - t.resolve() - t.mu.Lock() - defer t.mu.Unlock() - - // t.underlying might have been set while we were locking + // avoid acquiring the lock if we can if t.stateHas(underlying) { u = t.underlying break @@ -634,6 +627,9 @@ func (n *Named) resolveUnderlying() { } seen[t] = len(seen) + t.resolve() + t.mu.Lock() + defer t.mu.Unlock() assert(t.fromRHS != nil || t.allowNilRHS) rhs = t.fromRHS diff --git a/src/go/types/named.go b/src/go/types/named.go index b436d073e6..79f6fc6a49 100644 --- a/src/go/types/named.go +++ b/src/go/types/named.go @@ -565,7 +565,10 @@ func (n *Named) Underlying() Type { } } - n.resolveUnderlying() + if !n.stateHas(underlying) { + n.resolveUnderlying() + } + return n.underlying } @@ -591,11 +594,6 @@ func (t *Named) String() string { return TypeString(t, nil) } func (n *Named) resolveUnderlying() { assert(n.stateHas(resolved)) - // optimization for likely case - if n.stateHas(underlying) { - return - } - var seen map[*Named]int // allocated lazily var u Type for rhs := Type(n); u == nil; { @@ -621,12 +619,7 @@ func (n *Named) resolveUnderlying() { break } - // acquire the lock without checking; performance is probably fine - t.resolve() - t.mu.Lock() - defer t.mu.Unlock() - - // t.underlying might have been set while we were locking + // avoid acquiring the lock if we can if t.stateHas(underlying) { u = t.underlying break @@ -637,6 +630,9 @@ func (n *Named) resolveUnderlying() { } seen[t] = len(seen) + t.resolve() + t.mu.Lock() + defer t.mu.Unlock() assert(t.fromRHS != nil || t.allowNilRHS) rhs = t.fromRHS -- 2.52.0