Skip to content

Commit ea4631c

Browse files
rscgopherbot
authored andcommitted
internal/godebug: define more efficient API
We have been expanding our use of GODEBUG for compatibility, and the current implementation forces a tradeoff between freshness and efficiency. It parses the environment variable in full each time it is called, which is expensive. But if clients cache the result, they won't respond to run-time GODEBUG changes, as happened with x509sha1 (#56436). This CL changes the GODEBUG API to provide efficient, up-to-date results. Instead of a single Get function, New returns a *godebug.Setting that itself has a Get method. Clients can save the result of New, which is no more expensive than errors.New, in a global variable, and then call that variable's Get method to get the value. Get costs only two atomic loads in the case where the variable hasn't changed since the last call. Unfortunately, these changes do require importing sync from godebug, which will mean that sync itself will never be able to use a GODEBUG setting. That doesn't seem like such a hardship. If it was really necessary, the runtime could pass a setting to package sync itself at startup, with the caveat that that setting, like the ones used by runtime itself, would not respond to run-time GODEBUG changes. Change-Id: I99a3acfa24fb2a692610af26a5d14bbc62c966ac Reviewed-on: https://go-review.googlesource.com/c/go/+/449504 Run-TryBot: Russ Cox <[email protected]> Auto-Submit: Russ Cox <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 40bdcbb commit ea4631c

File tree

23 files changed

+245
-98
lines changed

23 files changed

+245
-98
lines changed

src/cmd/go/go_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,9 +2348,11 @@ func TestUpxCompression(t *testing.T) {
23482348
}
23492349
}
23502350

2351+
var gocacheverify = godebug.New("gocacheverify")
2352+
23512353
func TestCacheListStale(t *testing.T) {
23522354
tooSlow(t)
2353-
if godebug.Get("gocacheverify") == "1" {
2355+
if gocacheverify.Value() == "1" {
23542356
t.Skip("GODEBUG gocacheverify")
23552357
}
23562358
tg := testgo(t)
@@ -2373,7 +2375,7 @@ func TestCacheListStale(t *testing.T) {
23732375
func TestCacheCoverage(t *testing.T) {
23742376
tooSlow(t)
23752377

2376-
if godebug.Get("gocacheverify") == "1" {
2378+
if gocacheverify.Value() == "1" {
23772379
t.Skip("GODEBUG gocacheverify")
23782380
}
23792381

@@ -2407,7 +2409,7 @@ func TestIssue22588(t *testing.T) {
24072409

24082410
func TestIssue22531(t *testing.T) {
24092411
tooSlow(t)
2410-
if godebug.Get("gocacheverify") == "1" {
2412+
if gocacheverify.Value() == "1" {
24112413
t.Skip("GODEBUG gocacheverify")
24122414
}
24132415
tg := testgo(t)
@@ -2436,7 +2438,7 @@ func TestIssue22531(t *testing.T) {
24362438

24372439
func TestIssue22596(t *testing.T) {
24382440
tooSlow(t)
2439-
if godebug.Get("gocacheverify") == "1" {
2441+
if gocacheverify.Value() == "1" {
24402442
t.Skip("GODEBUG gocacheverify")
24412443
}
24422444
tg := testgo(t)
@@ -2466,7 +2468,7 @@ func TestIssue22596(t *testing.T) {
24662468
func TestTestCache(t *testing.T) {
24672469
tooSlow(t)
24682470

2469-
if godebug.Get("gocacheverify") == "1" {
2471+
if gocacheverify.Value() == "1" {
24702472
t.Skip("GODEBUG gocacheverify")
24712473
}
24722474
tg := testgo(t)

src/cmd/go/internal/fsys/fsys.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,27 +36,29 @@ func Trace(op, path string) {
3636
traceMu.Lock()
3737
defer traceMu.Unlock()
3838
fmt.Fprintf(traceFile, "%d gofsystrace %s %s\n", os.Getpid(), op, path)
39-
if traceStack != "" {
40-
if match, _ := pathpkg.Match(traceStack, path); match {
39+
if pattern := gofsystracestack.Value(); pattern != "" {
40+
if match, _ := pathpkg.Match(pattern, path); match {
4141
traceFile.Write(debug.Stack())
4242
}
4343
}
4444
}
4545

4646
var (
47-
doTrace bool
48-
traceStack string
49-
traceFile *os.File
50-
traceMu sync.Mutex
47+
doTrace bool
48+
traceFile *os.File
49+
traceMu sync.Mutex
50+
51+
gofsystrace = godebug.New("gofsystrace")
52+
gofsystracelog = godebug.New("gofsystracelog")
53+
gofsystracestack = godebug.New("gofsystracestack")
5154
)
5255

5356
func init() {
54-
if godebug.Get("gofsystrace") != "1" {
57+
if gofsystrace.Value() != "1" {
5558
return
5659
}
5760
doTrace = true
58-
traceStack = godebug.Get("gofsystracestack")
59-
if f := godebug.Get("gofsystracelog"); f != "" {
61+
if f := gofsystracelog.Value(); f != "" {
6062
// Note: No buffering on writes to this file, so no need to worry about closing it at exit.
6163
var err error
6264
traceFile, err = os.OpenFile(f, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0666)

src/cmd/go/internal/modindex/read.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import (
3737
// It will be removed before the release.
3838
// TODO(matloob): Remove enabled once we have more confidence on the
3939
// module index.
40-
var enabled bool = godebug.Get("goindex") != "0"
40+
var enabled = godebug.New("goindex").Value() != "0"
4141

4242
// Module represents and encoded module index file. It is used to
4343
// do the equivalent of build.Import of packages in the module and answer other
@@ -368,6 +368,8 @@ func relPath(path, modroot string) string {
368368
return str.TrimFilePathPrefix(filepath.Clean(path), filepath.Clean(modroot))
369369
}
370370

371+
var installgorootAll = godebug.New("installgoroot").Value() == "all"
372+
371373
// Import is the equivalent of build.Import given the information in Module.
372374
func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *build.Package, err error) {
373375
defer unprotect(protect(), &err)
@@ -436,7 +438,7 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b
436438
p.PkgTargetRoot = ctxt.joinPath(p.Root, pkgtargetroot)
437439

438440
// Set the install target if applicable.
439-
if !p.Goroot || (strings.EqualFold(godebug.Get("installgoroot"), "all") && p.ImportPath != "unsafe" && p.ImportPath != "builtin") {
441+
if !p.Goroot || (installgorootAll && p.ImportPath != "unsafe" && p.ImportPath != "builtin") {
440442
p.PkgObj = ctxt.joinPath(p.Root, pkga)
441443
}
442444
}

src/crypto/x509/x509.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,8 @@ func signaturePublicKeyAlgoMismatchError(expectedPubKeyAlgo PublicKeyAlgorithm,
813813
return fmt.Errorf("x509: signature algorithm specifies an %s public key, but have public key of type %T", expectedPubKeyAlgo.String(), pubKey)
814814
}
815815

816+
var x509sha1 = godebug.New("x509sha1")
817+
816818
// checkSignature verifies that signature is a valid signature over signed from
817819
// a crypto.PublicKey.
818820
func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey crypto.PublicKey, allowSHA1 bool) (err error) {
@@ -835,7 +837,7 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey
835837
return InsecureAlgorithmError(algo)
836838
case crypto.SHA1:
837839
// SHA-1 signatures are mostly disabled. See go.dev/issue/41682.
838-
if !allowSHA1 && godebug.Get("x509sha1") != "1" {
840+
if !allowSHA1 && x509sha1.Value() != "1" {
839841
return InsecureAlgorithmError(algo)
840842
}
841843
fallthrough

src/go/build/build.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,8 @@ func nameExt(name string) string {
521521
return name[i:]
522522
}
523523

524+
var installgoroot = godebug.New("installgoroot")
525+
524526
// Import returns details about the Go package named by the import path,
525527
// interpreting local import paths relative to the srcDir directory.
526528
// If the path is a local import path naming a package that can be imported
@@ -783,7 +785,7 @@ Found:
783785
p.PkgTargetRoot = ctxt.joinPath(p.Root, pkgtargetroot)
784786

785787
// Set the install target if applicable.
786-
if !p.Goroot || (strings.EqualFold(godebug.Get("installgoroot"), "all") && p.ImportPath != "unsafe" && p.ImportPath != "builtin") {
788+
if !p.Goroot || (installgoroot.Value() == "all" && p.ImportPath != "unsafe" && p.ImportPath != "builtin") {
787789
p.PkgObj = ctxt.joinPath(p.Root, pkga)
788790
}
789791
}

src/go/build/deps_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,10 @@ var depsRules = `
5252
internal/goarch, unsafe
5353
< internal/abi;
5454
55-
unsafe
56-
< internal/godebug;
57-
5855
# RUNTIME is the core runtime group of packages, all of them very light-weight.
5956
internal/abi, internal/cpu, internal/goarch,
6057
internal/coverage/rtcov, internal/goexperiment,
61-
internal/goos, internal/godebug, unsafe
58+
internal/goos, unsafe
6259
< internal/bytealg
6360
< internal/itoa
6461
< internal/unsafeheader
@@ -70,6 +67,7 @@ var depsRules = `
7067
< sync/atomic
7168
< internal/race
7269
< sync
70+
< internal/godebug
7371
< internal/reflectlite
7472
< errors
7573
< internal/oserror, math/bits

src/internal/cpu/cpu_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestDisableAllCapabilities(t *testing.T) {
4848
func TestAllCapabilitiesDisabled(t *testing.T) {
4949
MustHaveDebugOptionsSupport(t)
5050

51-
if godebug.Get("cpu.all") != "off" {
51+
if godebug.New("cpu.all").Value() != "off" {
5252
t.Skipf("skipping test: GODEBUG=cpu.all=off not set")
5353
}
5454

src/internal/cpu/cpu_x86_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestDisableSSE3(t *testing.T) {
2828
func TestSSE3DebugOption(t *testing.T) {
2929
MustHaveDebugOptionsSupport(t)
3030

31-
if godebug.Get("cpu.sse3") != "off" {
31+
if godebug.New("cpu.sse3").Value() != "off" {
3232
t.Skipf("skipping test: GODEBUG=cpu.sse3=off not set")
3333
}
3434

src/internal/fuzz/fuzz.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"reflect"
2222
"runtime"
2323
"strings"
24-
"sync"
2524
"time"
2625
)
2726

@@ -1077,14 +1076,8 @@ var zeroVals []any = []any{
10771076
uint64(0),
10781077
}
10791078

1080-
var (
1081-
debugInfo bool
1082-
debugInfoOnce sync.Once
1083-
)
1079+
var debugInfo = godebug.New("fuzzdebug").Value() == "1"
10841080

10851081
func shouldPrintDebugInfo() bool {
1086-
debugInfoOnce.Do(func() {
1087-
debugInfo = godebug.Get("fuzzdebug") == "1"
1088-
})
10891082
return debugInfo
10901083
}

src/internal/godebug/export_test.go

Lines changed: 0 additions & 7 deletions
This file was deleted.

0 commit comments

Comments
 (0)