unique: use a bespoke canonicalization map and runtime.AddCleanup

This change moves the unique package away from using a concurrent map
and instead toward a bespoke concurrent canonicalization map. The map
holds all its keys weakly, though keys may be looked up by value. The
result is the strong pointer for the canonical value. Entries in the map
are automatically cleaned up once the canonical reference no longer
exists.

Why do this? There's a problem with the current implementation when it
comes to chains of unique.Handle: because the unique map will have a
unique.Handle stored in its keys, each nested handle must be cleaned up
1 GC at a time. It takes N GC cycles, at minimum, to clean up a nested
chain of N handles. This implementation, where the *only* value in the
set is weakly-held, does not have this problem. The entire chain is
dropped at once.

The canon map implementation is a stripped-down version of HashTrieMap.
The weak set implementation also has lower memory overheads by virtue of
the fact that keys are all stored weakly. Whereas the previous map had
both a T and a weak.Pointer[T], this *only* has a weak.Pointer[T].

The canonicalization map is a better abstraction overall and
dramatically simplifies the unique.Make code.

While we're here, delete the background goroutine and switch to
runtime.AddCleanup. This is a step toward fixing #71772. We still need
some kind of back-pressure mechanism, which will be implemented in a
follow-up CL.

For #71772.
Fixes #71846.

Change-Id: I5b2ee04ebfc7f6dd24c2c4a959dd0f6a8af24ca4
Reviewed-on: https://go-review.googlesource.com/c/go/+/650256
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: David Chase <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>

Author: mknyszek

src/runtime/mfinal.go

@@ -324,7 +324,7 @@ func isGoPointerWithoutSpan(p unsafe.Pointer) bool {
 // blockUntilEmptyFinalizerQueue blocks until either the finalizer
 // queue is emptied (and the finalizers have executed) or the timeout
 // is reached. Returns true if the finalizer queue was emptied.
-// This is used by the runtime and sync tests.
+// This is used by the runtime, sync, and unique tests.
 func blockUntilEmptyFinalizerQueue(timeout int64) bool {
 	start := nanotime()
 	for nanotime()-start < timeout {
@@ -342,6 +342,11 @@ func blockUntilEmptyFinalizerQueue(timeout int64) bool {
 	return false
 }
 
+//go:linkname unique_runtime_blockUntilEmptyFinalizerQueue unique.runtime_blockUntilEmptyFinalizerQueue
+func unique_runtime_blockUntilEmptyFinalizerQueue(timeout int64) bool {
+	return blockUntilEmptyFinalizerQueue(timeout)
+}
+
 // SetFinalizer sets the finalizer associated with obj to the provided
 // finalizer function. When the garbage collector finds an unreachable block
 // with an associated finalizer, it clears the association and runs

src/runtime/mgc.go

@@ -1835,8 +1835,7 @@ func gcResetMarkState() {
 // Hooks for other packages
 
 var poolcleanup func()
-var boringCaches []unsafe.Pointer  // for crypto/internal/boring
-var uniqueMapCleanup chan struct{} // for unique
+var boringCaches []unsafe.Pointer // for crypto/internal/boring
 
 // sync_runtime_registerPoolCleanup should be an internal detail,
 // but widely used packages access it using linkname.
@@ -1857,22 +1856,6 @@ func boring_registerCache(p unsafe.Pointer) {
 	boringCaches = append(boringCaches, p)
 }
 
-//go:linkname unique_runtime_registerUniqueMapCleanup unique.runtime_registerUniqueMapCleanup
-func unique_runtime_registerUniqueMapCleanup(f func()) {
-	// Create the channel on the system stack so it doesn't inherit the current G's
-	// synctest bubble (if any).
-	systemstack(func() {
-		uniqueMapCleanup = make(chan struct{}, 1)
-	})
-	// Start the goroutine in the runtime so it's counted as a system goroutine.
-	go func(cleanup func()) {
-		for {
-			<-uniqueMapCleanup
-			cleanup()
-		}
-	}(f)
-}
-
 func clearpools() {
 	// clear sync.Pools
 	if poolcleanup != nil {
@@ -1884,14 +1867,6 @@ func clearpools() {
 		atomicstorep(p, nil)
 	}
 
-	// clear unique maps
-	if uniqueMapCleanup != nil {
-		select {
-		case uniqueMapCleanup <- struct{}{}:
-		default:
-		}
-	}
-
 	// Clear central sudog cache.
 	// Leave per-P caches alone, they have strictly bounded size.
 	// Disconnect cached list before dropping it on the floor,