From d367ec6a0ed0c016603c8aba697710a131a70db8 Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Tue, 1 Aug 2023 13:54:32 -0400
Subject: [PATCH] runtime: move pcvalue cache to M

Currently, the pcvalue cache is stack allocated for each operation
that needs to look up a lot of pcvalues. It's not always clear where
to put it, a lot of the time we just pass a nil cache, it doesn't get
reused across operations, and we put a surprising amount of effort
into threading these caches around.

This CL moves it to the M, where it can be long-lived and used by all
pcvalue lookups, and we don't have to carefully thread it across
operations.

Change-Id: I675e583e0daac887c8ef77a402ba792648d96027
Reviewed-on: https://go-review.googlesource.com/c/go/+/515276
Run-TryBot: Austin Clements <austin@google.com>
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
---
 src/runtime/runtime2.go |  3 +++
 src/runtime/symtab.go   | 46 +++++++++++++++++++++--------------------
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index 54fab050ea..e7db4dcdd7 100644
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -611,6 +611,9 @@ type m struct {
 	// Whether this is a pending preemption signal on this M.
 	signalPending atomic.Uint32
 
+	// pcvalue lookup cache
+	pcvalueCache pcvalueCache
+
 	dlogPerM
 
 	mOS
diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go
index ff5f5f7f0e..18ba683d69 100644
--- a/src/runtime/symtab.go
+++ b/src/runtime/symtab.go
@@ -843,30 +843,32 @@ func pcvalueCacheKey(targetpc uintptr) uintptr {
 }
 
 // Returns the PCData value, and the PC where this value starts.
-func pcvalue(f funcInfo, off uint32, targetpc uintptr, cache *pcvalueCache, strict bool) (int32, uintptr) {
+func pcvalue(f funcInfo, off uint32, targetpc uintptr, _ *pcvalueCache, strict bool) (int32, uintptr) {
 	if off == 0 {
 		return -1, 0
 	}
 
 	// Check the cache. This speeds up walks of deep stacks, which
-	// tend to have the same recursive functions over and over.
-	//
-	// This cache is small enough that full associativity is
-	// cheaper than doing the hashing for a less associative
-	// cache.
-	if cache != nil {
-		x := pcvalueCacheKey(targetpc)
-		for i := range cache.entries[x] {
+	// tend to have the same recursive functions over and over,
+	// or repetitive stacks between goroutines.
+	ck := pcvalueCacheKey(targetpc)
+	{
+		mp := acquirem()
+		cache := &mp.pcvalueCache
+		for i := range cache.entries[ck] {
 			// We check off first because we're more
 			// likely to have multiple entries with
 			// different offsets for the same targetpc
 			// than the other way around, so we'll usually
 			// fail in the first clause.
-			ent := &cache.entries[x][i]
+			ent := &cache.entries[ck][i]
 			if ent.off == off && ent.targetpc == targetpc {
-				return ent.val, ent.valPC
+				val, pc := ent.val, ent.valPC
+				releasem(mp)
+				return val, pc
 			}
 		}
+		releasem(mp)
 	}
 
 	if !f.valid() {
@@ -894,18 +896,18 @@ func pcvalue(f funcInfo, off uint32, targetpc uintptr, cache *pcvalueCache, stri
 			// larger than the cache.
 			// Put the new element at the beginning,
 			// since it is the most likely to be newly used.
-			if cache != nil {
-				x := pcvalueCacheKey(targetpc)
-				e := &cache.entries[x]
-				ci := fastrandn(uint32(len(cache.entries[x])))
-				e[ci] = e[0]
-				e[0] = pcvalueCacheEnt{
-					targetpc: targetpc,
-					off:      off,
-					val:      val,
-					valPC:    prevpc,
-				}
+			mp := acquirem()
+			cache := &mp.pcvalueCache
+			e := &cache.entries[ck]
+			ci := fastrandn(uint32(len(cache.entries[ck])))
+			e[ci] = e[0]
+			e[0] = pcvalueCacheEnt{
+				targetpc: targetpc,
+				off:      off,
+				val:      val,
+				valPC:    prevpc,
 			}
+			releasem(mp)
 
 			return val, prevpc
 		}
-- 
2.50.0