Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zap.Any add benchmarks #1311

Merged
merged 6 commits into from
Aug 1, 2023
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions logger_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ package zap

import (
"errors"
"runtime"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -238,3 +240,132 @@ func Benchmark100Fields(b *testing.B) {
logger.With(first...).Info("Child loggers with lots of context.", second...)
}
}

func dummy(wg *sync.WaitGroup, s string, i int) string {
if i == 0 {
wg.Wait()
return "1" + s
}
return dummy(wg, s, i-1)
}

// increaseAvgStack starts a background goroutine with a variable
// stack size. The goal is to move the average stack size higher,
// since https://go-review.googlesource.com/c/go/+/345889 this affects
// goroutine starting stack size.
func increaseAvgStack(n int) *sync.WaitGroup {
wg := sync.WaitGroup{}
wg.Add(1)

go dummy(&wg, "hi", n)
return &wg
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use a simple var data[8000]byte to grow the stack size.

func growAvgStack() {
	var wg sync.WaitGroup
	block := make(chan struct{})
	for i := 0; i < 1000; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()

			// Effectively the minimum size for the starting goroutine size.
			var data [8000]byte
			<-block
			runtime.KeepAlive(data)
		}()
	}

	close(block)
	wg.Wait()
}

Verified that it works by reading + printing the starting stack size of metrics.Sample{Name: "/gc/stack/starting-size:bytes"}

https://go.dev/play/p/3MFl3S7Zz-A


func BenchmarkAny(b *testing.B) {
key := "some-long-string-longer-than-16"

tests := []struct {
name string
typed func() Field
anyArg any
}{
{
name: "string",
typed: func() Field { return String(key, "yet-another-long-string") },
anyArg: "yet-another-long-string",
},
{
name: "stringer",
typed: func() Field { return Stringer(key, InfoLevel) },
anyArg: InfoLevel,
},
}

for _, tt := range tests {
b.Run(tt.name, func(b *testing.B) {
b.Run("field-only", func(b *testing.B) {
b.Run("typed", func(b *testing.B) {
withBenchedLogger(b, func(log *Logger) {
f := tt.typed()
runtime.KeepAlive(f)
})
})
b.Run("any", func(b *testing.B) {
withBenchedLogger(b, func(log *Logger) {
f := Any(key, tt.anyArg)
runtime.KeepAlive(f)
})
})
})
b.Run("log", func(b *testing.B) {
b.Run("typed", func(b *testing.B) {
withBenchedLogger(b, func(log *Logger) {
log.Info("", tt.typed())
})
})
b.Run("any", func(b *testing.B) {
withBenchedLogger(b, func(log *Logger) {
log.Info("", Any(key, tt.anyArg))
})
})
})
b.Run("log-go", func(b *testing.B) {
b.Run("typed", func(b *testing.B) {
wg := sync.WaitGroup{}
wg.Add(b.N)
b.ResetTimer()
withBenchedLogger(b, func(log *Logger) {
go func() {
log.Info("", tt.typed())
wg.Done()
}()
})
wg.Wait()
})
b.Run("any", func(b *testing.B) {
wg := sync.WaitGroup{}
wg.Add(b.N)
b.ResetTimer()
withBenchedLogger(b, func(log *Logger) {
go func() {
log.Info("", Any(key, tt.anyArg))
wg.Done()
}()
})
wg.Wait()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this works? (Related to the prior comment about b.N goroutines.)
Each iteration (whether with RunParallel or with a plain for loop), will last only as long as spawning the goroutine takes. I'm not sure this will do what we want?
I know we're trying to measure the cost inside a new goroutine, but this doesn't seem right?
Maybe we need to spawn and wait for the goroutine in each iteration of the loop?

var wg sync.WaitGroup
wg.Add(1)
go func() {
    ...
    wg.Done()
}()
wg.Wait()

This will measure the cost of the whole operation (including spawning the goroutine) per iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm.

It was testing something reasonable, because the changes we were making to the stack were clearly having an effect. I agree that this is better though, thanks.

})
})
// The stack growing below simulates production setup where some other
// goroutines exist and affect the starting goroutine stack size up.
// Otherwise, for tests with 2+ goroutines, the cost of starting the goroutine
// dominates and the cost of `any` stack overallocation is not visible.
b.Run("log-go-stack", func(b *testing.B) {
defer increaseAvgStack(5000).Done()
b.ResetTimer()
b.Run("typed", func(b *testing.B) {
wg := sync.WaitGroup{}
wg.Add(b.N)
withBenchedLogger(b, func(log *Logger) {
go func() {
log.Info("", tt.typed())
wg.Done()
}()
})
wg.Wait()
})
b.Run("any", func(b *testing.B) {
wg := sync.WaitGroup{}
wg.Add(b.N)
withBenchedLogger(b, func(log *Logger) {
go func() {
log.Info("", Any(key, tt.anyArg))
wg.Done()
}()
})
wg.Wait()
})
b.StopTimer()
})
})
}
}