go/types, types2: reduce locks held at once in resolveUnderlying

There is no need to hold locks for the entire chain of Named types in
resolveUnderlying. This change moves the locking / unlocking right to
where t.underlying is set.

This change consolidates logic into resolveUnderlying where possible
and makes minor stylistic / documentation adjustments.

Change-Id: Ic5ec5a7e9a0da8bc34954bf456e4e23a28df296d
Reviewed-on: https://go-review.googlesource.com/c/go/+/714403
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Mark Freeman 2025-10-23 16:48:00 -04:00 committed by Gopher Robot
parent 24af441437
commit b8aa1ee442
2 changed files with 32 additions and 34 deletions

View file

@ -590,7 +590,7 @@ func (n *Named) Underlying() Type {
}
}
if !n.stateHas(hasUnder) {
if !n.stateHas(hasUnder) { // minor performance optimization
n.resolveUnderlying()
}
@ -605,7 +605,8 @@ func (t *Named) String() string { return TypeString(t, nil) }
// TODO(rfindley): reorganize the loading and expansion methods under this
// heading.
// resolveUnderlying computes the underlying type of n.
// resolveUnderlying computes the underlying type of n. If n already has an
// underlying type, nothing happens.
//
// It does so by following RHS type chains for alias and named types. If any
// other type T is found, each named type in the chain has its underlying
@ -636,14 +637,13 @@ func (n *Named) resolveUnderlying() {
path[j] = t.obj
}
path = path[i:]
// Note: This code may only be called during type checking,
// hence n.check != nil.
// only called during type checking, hence n.check != nil
n.check.cycleError(path, firstInSrc(path))
u = Typ[Invalid]
break
}
// avoid acquiring the lock if we can
// don't recalculate the underlying
if t.stateHas(hasUnder) {
u = t.underlying
break
@ -655,9 +655,6 @@ func (n *Named) resolveUnderlying() {
seen[t] = len(seen)
t.unpack()
t.mu.Lock()
defer t.mu.Unlock()
assert(t.rhs() != nil || t.allowNilRHS)
rhs = t.rhs()
@ -666,17 +663,19 @@ func (n *Named) resolveUnderlying() {
}
}
// set underlying for all Named types in the chain
for t := range seen {
func() {
t.mu.Lock()
defer t.mu.Unlock()
// Careful, t.underlying has lock-free readers. Since we might be racing
// another call to resolveUnderlying, we have to avoid overwriting
// t.underlying. Otherwise, the race detector will be tripped.
if t.stateHas(hasUnder) {
continue
}
if !t.stateHas(hasUnder) {
t.underlying = u
t.setState(hasUnder)
}
}()
}
}
func (n *Named) lookupMethod(pkg *Package, name string, foldCase bool) (int, *Func) {

View file

@ -593,7 +593,7 @@ func (n *Named) Underlying() Type {
}
}
if !n.stateHas(hasUnder) {
if !n.stateHas(hasUnder) { // minor performance optimization
n.resolveUnderlying()
}
@ -608,7 +608,8 @@ func (t *Named) String() string { return TypeString(t, nil) }
// TODO(rfindley): reorganize the loading and expansion methods under this
// heading.
// resolveUnderlying computes the underlying type of n.
// resolveUnderlying computes the underlying type of n. If n already has an
// underlying type, nothing happens.
//
// It does so by following RHS type chains for alias and named types. If any
// other type T is found, each named type in the chain has its underlying
@ -639,14 +640,13 @@ func (n *Named) resolveUnderlying() {
path[j] = t.obj
}
path = path[i:]
// Note: This code may only be called during type checking,
// hence n.check != nil.
// only called during type checking, hence n.check != nil
n.check.cycleError(path, firstInSrc(path))
u = Typ[Invalid]
break
}
// avoid acquiring the lock if we can
// don't recalculate the underlying
if t.stateHas(hasUnder) {
u = t.underlying
break
@ -658,9 +658,6 @@ func (n *Named) resolveUnderlying() {
seen[t] = len(seen)
t.unpack()
t.mu.Lock()
defer t.mu.Unlock()
assert(t.rhs() != nil || t.allowNilRHS)
rhs = t.rhs()
@ -669,17 +666,19 @@ func (n *Named) resolveUnderlying() {
}
}
// set underlying for all Named types in the chain
for t := range seen {
func() {
t.mu.Lock()
defer t.mu.Unlock()
// Careful, t.underlying has lock-free readers. Since we might be racing
// another call to resolveUnderlying, we have to avoid overwriting
// t.underlying. Otherwise, the race detector will be tripped.
if t.stateHas(hasUnder) {
continue
}
if !t.stateHas(hasUnder) {
t.underlying = u
t.setState(hasUnder)
}
}()
}
}
func (n *Named) lookupMethod(pkg *Package, name string, foldCase bool) (int, *Func) {