runtime: remove GODEBUG=runtimecontentionstacks

Go 1.22 promised to remove the setting in a future release once the
semantics of runtime-internal lock contention matched that of
sync.Mutex. That work is done, remove the setting.

Previously reviewed as https://go.dev/cl/585639.

For #66999

Change-Id: I9fe62558ba0ac12824874a0bb1b41efeb7c0853f
Reviewed-on: https://go-review.googlesource.com/c/go/+/668995
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/doc/godebug.md b/doc/godebug.md
index 23427c85b5793c00eefecb851e2efb46cbb21301..1d6e6d78d07759e41278fec0d0650d6b72d93037 100644 (file)
--- a/doc/godebug.md
+++ b/doc/godebug.md
@@ -169,6 +169,9 @@ Go command will follow symlinks to regular files embedding files.
 The default value `embedfollowsymlinks=0` does not allow following
 symlinks. `embedfollowsymlinks=1` will allow following symlinks.
 
+Go 1.25 corrected the semantics of contention reports for runtime-internal locks,
+and so removed the [`runtimecontentionstacks` setting](/pkg/runtime#hdr-Environment_Variable).
+
 ### Go 1.24
 
 Go 1.24 added a new `fips140` setting that controls whether the Go

diff --git a/doc/next/6-stdlib/99-minor/runtime/pprof/66999.md b/doc/next/6-stdlib/99-minor/runtime/pprof/66999.md
new file mode 100644 (file)
index 0000000..f222628
--- /dev/null
+++ b/doc/next/6-stdlib/99-minor/runtime/pprof/66999.md
@@ -0,0 +1,6 @@
+The mutex profile for contention on runtime-internal locks now correctly points
+to the end of the critical section that caused the delay. This matches the
+profile's behavior for contention on `sync.Mutex` values. The
+`runtimecontentionstacks` setting for `GODEBUG`, which allowed opting in to the
+unusual behavior of Go 1.22 through 1.24 for this part of the profile, is now
+gone.

diff --git a/src/runtime/extern.go b/src/runtime/extern.go
index 8ee89ab94fd1cb974d0c5e5f93336e0720aa240f..eb30b0566ecb3661e8f7e5d1237f9dfd467182e0 100644 (file)
--- a/src/runtime/extern.go
+++ b/src/runtime/extern.go
@@ -166,15 +166,6 @@ 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 178dd968f72b56a50c6283ca967829060b13739d..5fc022efc6cf1d844b85be0b9162e3e3d8c5087e 100644 (file)
--- a/src/runtime/metrics_test.go
+++ b/src/runtime/metrics_test.go
@@ -6,7 +6,6 @@ package runtime_test
 
 import (
        "bytes"
-       "fmt"
        "internal/abi"
        "internal/goexperiment"
        "internal/profile"
@@ -955,17 +954,6 @@ 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 {
@@ -1229,9 +1217,9 @@ func TestRuntimeLockMetricsAndProfile(t *testing.T) {
 
                        stks[i] = []string{
                                "runtime.unlock",
-                               "runtime_test." + name + ".func5.1.1",
+                               "runtime_test." + name + ".func4.1.1",
                                marker,
-                               "runtime_test." + name + ".func5.1",
+                               "runtime_test." + name + ".func4.1",
                                "runtime_test.(*contentionWorker).run",
                        }
                }
@@ -1411,14 +1399,14 @@ func TestRuntimeLockMetricsAndProfile(t *testing.T) {
                        {
                                "runtime.unlock",
                                "runtime.semrelease1",
-                               "runtime_test.TestRuntimeLockMetricsAndProfile.func6.1",
+                               "runtime_test.TestRuntimeLockMetricsAndProfile.func5.1",
                                "runtime_test.(*contentionWorker).run",
                        },
                        {
                                "runtime.unlock",
                                "runtime.semacquire1",
                                "runtime.semacquire",
-                               "runtime_test.TestRuntimeLockMetricsAndProfile.func6.1",
+                               "runtime_test.TestRuntimeLockMetricsAndProfile.func5.1",
                                "runtime_test.(*contentionWorker).run",
                        },
                }

diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go
index f80c8418ac01fd372f52863be99dbce87cad6293..5e2643600dc040410591e4b3436492a38a887ada 100644 (file)
--- a/src/runtime/mprof.go
+++ b/src/runtime/mprof.go
@@ -732,11 +732,6 @@ func (prof *mLockProfile) captureStack() {
        prof.haveStack = true
 
        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 b7680a13fdcb2589ed7a4be0fb03b26764c28c78..d295991ef7819358bc0559a71258863e7e90cac6 100644 (file)
--- a/src/runtime/pprof/pprof.go
+++ b/src/runtime/pprof/pprof.go
@@ -169,12 +169,6 @@ 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 ec4f7d043393f42f853a89fef3b8b67794165d86..2132ceecb2f0869249572a2c95cfd77f1537bde0 100644 (file)
--- a/src/runtime/runtime1.go
+++ b/src/runtime/runtime1.go
@@ -320,7 +320,6 @@ var debug struct {
        gctrace                  int32
        invalidptr               int32
        madvdontneed             int32 // for Linux; issue 28466
-       runtimeContentionStacks  atomic.Int32
        scavtrace                int32
        scheddetail              int32
        schedtrace               int32
@@ -385,7 +384,6 @@ 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},