From 73532c11b075de15d12f8c813405f52395fd0caf Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Fri, 17 Apr 2015 21:29:50 -0400 Subject: [PATCH 1/2] Reduce allocations performed by the Logger returned from log.With. The old method performed O(n) allocs, the new method is O(1), where n is the number of nested With contexts. --- log/log.go | 41 +++++++++++++++++++++++++++++++++-------- log/log_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/log/log.go b/log/log.go index 155e19969..960c8d579 100644 --- a/log/log.go +++ b/log/log.go @@ -23,14 +23,32 @@ func With(logger Logger, keyvals ...interface{}) Logger { if w, ok := logger.(Wither); ok { return w.With(keyvals...) } - return LoggerFunc(func(kvs ...interface{}) error { - // Limiting the capacity of the first argument to append ensures that - // a new backing array is created if the slice must grow. Using the - // extra capacity without copying risks a data race that would violate - // the Logger interface contract. - n := len(keyvals) - return logger.Log(append(keyvals[:n:n], kvs...)...) - }) + // Limiting the capacity of the stored keyvals ensures that a new + // backing array is created if the slice must grow in Log or With. + // Using the extra capacity without copying risks a data race that + // would violate the Logger interface contract. + n := len(keyvals) + return &withLogger{ + logger: logger, + keyvals: keyvals[:n:n], + } +} + +type withLogger struct { + logger Logger + keyvals []interface{} +} + +func (l *withLogger) Log(kvs ...interface{}) error { + return l.logger.Log(append(l.keyvals, kvs...)...) +} + +func (l *withLogger) With(keyvals ...interface{}) Logger { + n := len(l.keyvals) + len(keyvals) + return &withLogger{ + logger: l.logger, + keyvals: append(l.keyvals, keyvals...)[:n:n], + } } // LoggerFunc is an adapter to allow use of ordinary functions as Loggers. If @@ -49,3 +67,10 @@ func (f LoggerFunc) Log(keyvals ...interface{}) error { type Wither interface { With(keyvals ...interface{}) Logger } + +// NewDiscardLogger returns a logger that does not log anything. +func NewDiscardLogger() Logger { + return LoggerFunc(func(...interface{}) error { + return nil + }) +} diff --git a/log/log_test.go b/log/log_test.go index f12721266..837a6f4de 100644 --- a/log/log_test.go +++ b/log/log_test.go @@ -78,3 +78,46 @@ func TestWithConcurrent(t *testing.T) { } } } + +func BenchmarkDiscard(b *testing.B) { + logger := log.NewDiscardLogger() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + logger.Log("k", "v") + } +} + +func BenchmarkOneWith(b *testing.B) { + logger := log.NewDiscardLogger() + logger = log.With(logger, "k", "v") + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + logger.Log("k", "v") + } +} + +func BenchmarkTwoWith(b *testing.B) { + logger := log.NewDiscardLogger() + for i := 0; i < 2; i++ { + logger = log.With(logger, "k", "v") + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + logger.Log("k", "v") + } +} + +func BenchmarkTenWith(b *testing.B) { + logger := log.NewDiscardLogger() + for i := 0; i < 10; i++ { + logger = log.With(logger, "k", "v") + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + logger.Log("k", "v") + } +} From 68ee34a42fac6708abf21a3fb0b09b7cf83e32fc Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Sat, 18 Apr 2015 19:42:49 -0400 Subject: [PATCH 2/2] Use consistent parameter names. --- log/log.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/log/log.go b/log/log.go index 960c8d579..5cf1b9abd 100644 --- a/log/log.go +++ b/log/log.go @@ -39,8 +39,8 @@ type withLogger struct { keyvals []interface{} } -func (l *withLogger) Log(kvs ...interface{}) error { - return l.logger.Log(append(l.keyvals, kvs...)...) +func (l *withLogger) Log(keyvals ...interface{}) error { + return l.logger.Log(append(l.keyvals, keyvals...)...) } func (l *withLogger) With(keyvals ...interface{}) Logger {