Revert "runtime: remove GODEBUG=runtimecontentionstacks"

This reverts commit 87e930f7289136fad1310d4b63dd4127e409bac5 (CL 585639)

Reason for revert: This is part of a patch series that changed the
handling of contended lock2/unlock2 calls, reducing the maximum
throughput of contended runtime.mutex values, and causing a performance
regression on applications where that is (or became) the bottleneck.

Updates #66999
Updates #67585

Change-Id: I1e286d2a16d16e4af202cd5dc04b2d9c4ee71b32
Reviewed-on: https://go-review.googlesource.com/c/go/+/589097
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>

5 files changed, 37 insertions, 3 deletions

diff --git a/src/runtime/extern.go b/src/runtime/extern.go
index 96efcf3273..2019be4dde 100644
--- a/src/runtime/extern.go
+++ b/src/runtime/extern.go
@@ -159,6 +159,15 @@ It is a comma-separated list of name=val pairs setting these named variables:
 	panicnil: setting panicnil=1 disables the runtime error when calling panic with nil
 	interface value or an untyped nil.
 
+	runtimecontentionstacks: setting runtimecontentionstacks=1 enables inclusion of call stacks
+	related to contention on runtime-internal locks in the "mutex" profile, subject to the
+	MutexProfileFraction setting. When runtimecontentionstacks=0, contention on
+	runtime-internal locks will report as "runtime._LostContendedRuntimeLock". When
+	runtimecontentionstacks=1, the call stacks will correspond to the unlock call that released
+	the lock. But instead of the value corresponding to the amount of contention that call
+	stack caused, it corresponds to the amount of time the caller of unlock had to wait in its
+	original call to lock. A future release is expected to align those and remove this setting.
+
 	invalidptr: invalidptr=1 (the default) causes the garbage collector and stack
 	copier to crash the program if an invalid pointer value (for example, 1)
 	is found in a pointer-typed location. Setting invalidptr=0 disables this check.
diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go
index fbf19e2b5e..ebbf0e4fd0 100644
--- a/src/runtime/metrics_test.go
+++ b/src/runtime/metrics_test.go
@@ -6,6 +6,7 @@ package runtime_test
 
 import (
 	"bytes"
+	"fmt"
 	"internal/abi"
 	"internal/goexperiment"
 	"internal/profile"
@@ -954,6 +955,17 @@ func TestRuntimeLockMetricsAndProfile(t *testing.T) {
 		t.Fatalf("need MutexProfileRate 0, got %d", old)
 	}
 
+	{
+		before := os.Getenv("GODEBUG")
+		for _, s := range strings.Split(before, ",") {
+			if strings.HasPrefix(s, "runtimecontentionstacks=") {
+				t.Logf("GODEBUG includes explicit setting %q", s)
+			}
+		}
+		defer func() { os.Setenv("GODEBUG", before) }()
+		os.Setenv("GODEBUG", fmt.Sprintf("%s,runtimecontentionstacks=1", before))
+	}
+
 	t.Logf("NumCPU %d", runtime.NumCPU())
 	t.Logf("GOMAXPROCS %d", runtime.GOMAXPROCS(0))
 	if minCPU := 2; runtime.NumCPU() < minCPU {
@@ -1152,7 +1164,7 @@ func TestRuntimeLockMetricsAndProfile(t *testing.T) {
 
 		stks := [][]string{{
 			"runtime.unlock",
-			"runtime_test." + name + ".func4.1",
+			"runtime_test." + name + ".func5.1",
 			"runtime_test.(*contentionWorker).run",
 		}}
 
@@ -1258,14 +1270,14 @@ func TestRuntimeLockMetricsAndProfile(t *testing.T) {
 			{
 				"runtime.unlock",
 				"runtime.semrelease1",
-				"runtime_test.TestRuntimeLockMetricsAndProfile.func5.1",
+				"runtime_test.TestRuntimeLockMetricsAndProfile.func6.1",
 				"runtime_test.(*contentionWorker).run",
 			},
 			{
 				"runtime.unlock",
 				"runtime.semacquire1",
 				"runtime.semacquire",
-				"runtime_test.TestRuntimeLockMetricsAndProfile.func5.1",
+				"runtime_test.TestRuntimeLockMetricsAndProfile.func6.1",
 				"runtime_test.(*contentionWorker).run",
 			},
 		}
diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go
index fd0a018724..93d49275c9 100644
--- a/src/runtime/mprof.go
+++ b/src/runtime/mprof.go
@@ -915,6 +915,11 @@ func (prof *mLockProfile) captureStack() {
 	}
 
 	prof.stack[0] = logicalStackSentinel
+	if debug.runtimeContentionStacks.Load() == 0 {
+		prof.stack[1] = abi.FuncPCABIInternal(_LostContendedRuntimeLock) + sys.PCQuantum
+		prof.stack[2] = 0
+		return
+	}
 
 	var nstk int
 	gp := getg()
diff --git a/src/runtime/pprof/pprof.go b/src/runtime/pprof/pprof.go
index 43ef66f0b0..be17e59875 100644
--- a/src/runtime/pprof/pprof.go
+++ b/src/runtime/pprof/pprof.go
@@ -166,6 +166,12 @@ import (
 // holds a lock for 1s while 5 other goroutines are waiting for the entire
 // second to acquire the lock, its unlock call stack will report 5s of
 // contention.
+//
+// Runtime-internal locks are always reported at the location
+// "runtime._LostContendedRuntimeLock". More detailed stack traces for
+// runtime-internal locks can be obtained by setting
+// `GODEBUG=runtimecontentionstacks=1` (see package [runtime] docs for
+// caveats).
 type Profile struct {
 	name  string
 	mu    sync.Mutex
diff --git a/src/runtime/runtime1.go b/src/runtime/runtime1.go
index c74f6d2c72..03ef74b8dc 100644
--- a/src/runtime/runtime1.go
+++ b/src/runtime/runtime1.go
@@ -319,6 +319,7 @@ var debug struct {
 	gctrace                  int32
 	invalidptr               int32
 	madvdontneed             int32 // for Linux; issue 28466
+	runtimeContentionStacks  atomic.Int32
 	scavtrace                int32
 	scheddetail              int32
 	schedtrace               int32
@@ -380,6 +381,7 @@ var dbgvars = []*dbgVar{
 	{name: "madvdontneed", value: &debug.madvdontneed},
 	{name: "panicnil", atomic: &debug.panicnil},
 	{name: "profstackdepth", value: &debug.profstackdepth, def: 128},
+	{name: "runtimecontentionstacks", atomic: &debug.runtimeContentionStacks},
 	{name: "sbrk", value: &debug.sbrk},
 	{name: "scavtrace", value: &debug.scavtrace},
 	{name: "scheddetail", value: &debug.scheddetail},