Skip to content

Commit a8d7201

Browse files
authored
log: avoid stack lookups when not needed/used (#28069)
Avoids the somewhat expensive stack.Caller invocation by checking if it is needed
1 parent c60f7dd commit a8d7201

File tree

5 files changed

+99
-5
lines changed

5 files changed

+99
-5
lines changed

common/types_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"reflect"
2626
"strings"
2727
"testing"
28+
"time"
2829
)
2930

3031
func TestBytesConversion(t *testing.T) {
@@ -583,3 +584,14 @@ func TestAddressEIP55(t *testing.T) {
583584
t.Fatal("Unexpected address after unmarshal")
584585
}
585586
}
587+
588+
func BenchmarkPrettyDuration(b *testing.B) {
589+
var x = PrettyDuration(time.Duration(int64(1203123912312)))
590+
b.Logf("Pre %s", time.Duration(x).String())
591+
var a string
592+
b.ResetTimer()
593+
for i := 0; i < b.N; i++ {
594+
a = x.String()
595+
}
596+
b.Logf("Post %s", a)
597+
}

log/format.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,16 @@ var locationTrims = []string{
3333
// format output.
3434
func PrintOrigins(print bool) {
3535
locationEnabled.Store(print)
36+
if print {
37+
stackEnabled.Store(true)
38+
}
3639
}
3740

41+
// stackEnabled is an atomic flag controlling whether the log handler needs
42+
// to store the callsite stack. This is needed in case any handler wants to
43+
// print locations (locationEnabled), use vmodule, or print full stacks (BacktraceAt).
44+
var stackEnabled atomic.Bool
45+
3846
// locationEnabled is an atomic flag controlling whether the terminal formatter
3947
// should append the log locations too when printing entries.
4048
var locationEnabled atomic.Bool

log/handler_glog.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ func (h *GlogHandler) Vmodule(ruleset string) error {
139139
h.patterns = filter
140140
h.siteCache = make(map[uintptr]Lvl)
141141
h.override.Store(len(filter) != 0)
142-
142+
// Enable location storage (globally)
143+
if len(h.patterns) > 0 {
144+
stackEnabled.Store(true)
145+
}
143146
return nil
144147
}
145148

@@ -172,7 +175,8 @@ func (h *GlogHandler) BacktraceAt(location string) error {
172175

173176
h.location = location
174177
h.backtrace.Store(len(location) > 0)
175-
178+
// Enable location storage (globally)
179+
stackEnabled.Store(true)
176180
return nil
177181
}
178182

log/logger.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,19 +177,22 @@ type logger struct {
177177
}
178178

179179
func (l *logger) write(msg string, lvl Lvl, ctx []interface{}, skip int) {
180-
l.h.Log(&Record{
180+
record := &Record{
181181
Time: time.Now(),
182182
Lvl: lvl,
183183
Msg: msg,
184184
Ctx: newContext(l.ctx, ctx),
185-
Call: stack.Caller(skip),
186185
KeyNames: RecordKeyNames{
187186
Time: timeKey,
188187
Msg: msgKey,
189188
Lvl: lvlKey,
190189
Ctx: ctxKey,
191190
},
192-
})
191+
}
192+
if stackEnabled.Load() {
193+
record.Call = stack.Caller(skip)
194+
}
195+
l.h.Log(record)
193196
}
194197

195198
func (l *logger) New(ctx ...interface{}) Logger {

log/logger_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package log
2+
3+
import (
4+
"bytes"
5+
"os"
6+
"strings"
7+
"testing"
8+
)
9+
10+
// TestLoggingWithTrace checks that if BackTraceAt is set, then the
11+
// gloghandler is capable of spitting out a stacktrace
12+
func TestLoggingWithTrace(t *testing.T) {
13+
defer stackEnabled.Store(stackEnabled.Load())
14+
out := new(bytes.Buffer)
15+
logger := New()
16+
{
17+
glog := NewGlogHandler(StreamHandler(out, TerminalFormat(false)))
18+
glog.Verbosity(LvlTrace)
19+
if err := glog.BacktraceAt("logger_test.go:24"); err != nil {
20+
t.Fatal(err)
21+
}
22+
logger.SetHandler(glog)
23+
}
24+
logger.Trace("a message", "foo", "bar") // Will be bumped to INFO
25+
have := out.String()
26+
if !strings.HasPrefix(have, "INFO") {
27+
t.Fatalf("backtraceat should bump level to info: %s", have)
28+
}
29+
// The timestamp is locale-dependent, so we want to trim that off
30+
// "INFO [01-01|00:00:00.000] a messag ..." -> "a messag..."
31+
have = strings.Split(have, "]")[1]
32+
wantPrefix := " a message\n\ngoroutine"
33+
if !strings.HasPrefix(have, wantPrefix) {
34+
t.Errorf("\nhave: %q\nwant: %q\n", have, wantPrefix)
35+
}
36+
}
37+
38+
// TestLoggingWithVmodule checks that vmodule works.
39+
func TestLoggingWithVmodule(t *testing.T) {
40+
defer stackEnabled.Store(stackEnabled.Load())
41+
out := new(bytes.Buffer)
42+
logger := New()
43+
{
44+
glog := NewGlogHandler(StreamHandler(out, TerminalFormat(false)))
45+
glog.Verbosity(LvlCrit)
46+
logger.SetHandler(glog)
47+
logger.Warn("This should not be seen", "ignored", "true")
48+
glog.Vmodule("logger_test.go=5")
49+
}
50+
logger.Trace("a message", "foo", "bar")
51+
have := out.String()
52+
// The timestamp is locale-dependent, so we want to trim that off
53+
// "INFO [01-01|00:00:00.000] a messag ..." -> "a messag..."
54+
have = strings.Split(have, "]")[1]
55+
want := " a message foo=bar\n"
56+
if have != want {
57+
t.Errorf("\nhave: %q\nwant: %q\n", have, want)
58+
}
59+
}
60+
61+
func BenchmarkTraceLogging(b *testing.B) {
62+
Root().SetHandler(LvlFilterHandler(LvlInfo, StreamHandler(os.Stderr, TerminalFormat(true))))
63+
b.ResetTimer()
64+
for i := 0; i < b.N; i++ {
65+
Trace("a message", "v", i)
66+
}
67+
}

0 commit comments

Comments
 (0)