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 <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>

diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go
index 99e27434840bedadaa6808cb0a79a49da785153a..26be3104b8c0bb5b378f4363ed2214ad509eecb4 100644 (file)
--- 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 b436d073e6dfa9cfbdb00cb025a7b71d3848269c..79f6fc6a494160ddedd0a640b715ac9549697f72 100644 (file)
--- 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