From 6654c8194680c25b7b88429cf7e1e3261b6046df Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Tue, 26 Feb 2019 03:26:48 -0500 Subject: [PATCH] log: fix remaining misuse of runtime.Callers/runtime.FuncForPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #17770. This commit fixes the last user of `runtime.Callers` that misused the stdlib function by translating the PC values it returned directly into symbolic information (see https://golang.org/pkg/runtime/#Callers). Go's documentation warns that this is a recipe for disaster when mixed with mid-stack inlining. The other concern in #17770 was this comment: https://github.com/cockroachdb/cockroach/issues/17770#issuecomment-332324377. This was discussed in https://github.com/golang/go/issues/29582 and addressed in https://github.com/golang/go/commit/956879dd0bf31b26d2425c2eadbeb19b90812187. An alternative would be to use `runtime.Caller` here, but that would force an allocation that was hard-earned in #29017. Instead, this commit avoids any performance hit. ``` name old time/op new time/op delta Header-4 267ns ± 1% 268ns ± 0% ~ (p=0.584 n=10+10) VDepthWithVModule-4 260ns ± 3% 255ns ± 1% -1.87% (p=0.018 n=10+9) name old alloc/op new alloc/op delta Header-4 0.00B 0.00B ~ (all equal) VDepthWithVModule-4 0.00B 0.00B ~ (all equal) name old allocs/op new allocs/op delta Header-4 0.00 0.00 ~ (all equal) VDepthWithVModule-4 0.00 0.00 ~ (all equal) ``` Release note: None --- pkg/util/log/clog.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/util/log/clog.go b/pkg/util/log/clog.go index ad9e48fb9337..86eda9dea127 100644 --- a/pkg/util/log/clog.go +++ b/pkg/util/log/clog.go @@ -1391,9 +1391,9 @@ func NewStdLogger(severity Severity) *stdLog.Logger { // of its .go suffix, and uses filepath.Match, which is a little more // general than the *? matching used in C++. // l.mu is held. -func (l *loggingT) setV(pc uintptr) level { - fn := runtime.FuncForPC(pc) - file, _ := fn.FileLine(pc) +func (l *loggingT) setV(pc [1]uintptr) level { + frame, _ := runtime.CallersFrames(pc[:]).Next() + file := frame.File // The file is something like /a/b/c/d.go. We want just the d. if strings.HasSuffix(file, ".go") { file = file[:len(file)-3] @@ -1403,11 +1403,11 @@ func (l *loggingT) setV(pc uintptr) level { } for _, filter := range l.vmodule.filter { if filter.match(file) { - l.vmap[pc] = filter.level + l.vmap[pc[0]] = filter.level return filter.level } } - l.vmap[pc] = 0 + l.vmap[pc[0]] = 0 return 0 } @@ -1477,7 +1477,7 @@ func VDepth(l int32, depth int) bool { logging.mu.Lock() v, ok := logging.vmap[pcs[0]] if !ok { - v = logging.setV(pcs[0]) + v = logging.setV(pcs) } logging.mu.Unlock() logging.pcsPool.Put(poolObj)