Skip to content

Commit 41c4c9b

Browse files
committed
log: avoid stack lookups when not needed/used (ethereum#28069)
1 parent 6b81e68 commit 41c4c9b

File tree

5 files changed

+99
-4
lines changed

5 files changed

+99
-4
lines changed

common/types_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"math/big"
2222
"strings"
2323
"testing"
24+
"time"
2425
)
2526

2627
func TestBytesConversion(t *testing.T) {
@@ -203,3 +204,14 @@ func TestStringToBinaryAddress(t *testing.T) {
203204
}
204205
}
205206
}
207+
208+
func BenchmarkPrettyDuration(b *testing.B) {
209+
var x = PrettyDuration(time.Duration(int64(1203123912312)))
210+
b.Logf("Pre %s", time.Duration(x).String())
211+
var a string
212+
b.ResetTimer()
213+
for i := 0; i < b.N; i++ {
214+
a = x.String()
215+
}
216+
b.Logf("Post %s", a)
217+
}

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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +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+
// Enable location storage (globally)
143+
if len(h.patterns) > 0 {
144+
stackEnabled.Store(true)
145+
}
142146

143147
return nil
144148
}
@@ -172,7 +176,8 @@ func (h *GlogHandler) BacktraceAt(location string) error {
172176

173177
h.location = location
174178
h.backtrace.Store(len(location) > 0)
175-
179+
// Enable location storage (globally)
180+
stackEnabled.Store(true)
176181
return nil
177182
}
178183

log/logger.go

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

178178
func (l *logger) write(msg string, lvl Lvl, ctx []interface{}) {
179-
l.h.Log(&Record{
179+
record := &Record{
180180
Time: time.Now(),
181181
Lvl: lvl,
182182
Msg: msg,
183183
Ctx: newContext(l.ctx, ctx),
184-
Call: stack.Caller(2),
185184
KeyNames: RecordKeyNames{
186185
Time: timeKey,
187186
Msg: msgKey,
188187
Lvl: lvlKey,
189188
Ctx: ctxKey,
190189
},
191-
})
190+
}
191+
if stackEnabled.Load() {
192+
record.Call = stack.Caller(2)
193+
}
194+
l.h.Log(record)
192195
}
193196

194197
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)