From f798dc682539df0bc60b30bedce9d6f5d5192d47 Mon Sep 17 00:00:00 2001 From: Carlo Alberto Ferraris Date: Thu, 1 Sep 2022 14:06:55 +0900 Subject: [PATCH] fmt: recycle printers with large buffers Previously when a printer had a large buffer we dropped both the buffer and the printer. There is no need to drop the printer in this case, as a printer with a nil buffer is valid. So we just drop the buffer and recycle the printer anyway. This saves one allocation in case the buffer is over the limit. Also tighten some of the tests for other unrelated cases. Change-Id: Iba1b6a71ca4691464b8c68ab0b6ab0d4d5d6168c Reviewed-on: https://go-review.googlesource.com/c/go/+/427395 Reviewed-by: Ian Lance Taylor Run-TryBot: Rob Pike Reviewed-by: Heschi Kreinick Reviewed-by: Rob Pike TryBot-Result: Gopher Robot --- src/fmt/fmt_test.go | 15 ++++++++++----- src/fmt/print.go | 14 ++++++++------ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/fmt/fmt_test.go b/src/fmt/fmt_test.go index aaeac3875a..d2fa81a7b3 100644 --- a/src/fmt/fmt_test.go +++ b/src/fmt/fmt_test.go @@ -1429,11 +1429,16 @@ var mallocTest = []struct { }{ {0, `Sprintf("")`, func() { Sprintf("") }}, {1, `Sprintf("xxx")`, func() { Sprintf("xxx") }}, - {2, `Sprintf("%x")`, func() { Sprintf("%x", 7) }}, - {2, `Sprintf("%s")`, func() { Sprintf("%s", "hello") }}, - {3, `Sprintf("%x %x")`, func() { Sprintf("%x %x", 7, 112) }}, - {2, `Sprintf("%g")`, func() { Sprintf("%g", float32(3.14159)) }}, // TODO: Can this be 1? - {1, `Fprintf(buf, "%s")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%s", "hello") }}, + {0, `Sprintf("%x")`, func() { Sprintf("%x", 7) }}, + {1, `Sprintf("%x")`, func() { Sprintf("%x", 1<<16) }}, + {3, `Sprintf("%80000s")`, func() { Sprintf("%80000s", "hello") }}, // large buffer (>64KB) + {1, `Sprintf("%s")`, func() { Sprintf("%s", "hello") }}, + {1, `Sprintf("%x %x")`, func() { Sprintf("%x %x", 7, 112) }}, + {1, `Sprintf("%g")`, func() { Sprintf("%g", float32(3.14159)) }}, + {0, `Fprintf(buf, "%s")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%s", "hello") }}, + {0, `Fprintf(buf, "%x")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%x", 7) }}, + {0, `Fprintf(buf, "%x")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%x", 1<<16) }}, + {2, `Fprintf(buf, "%80000s")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%80000s", "hello") }}, // large buffer (>64KB) // If the interface value doesn't need to allocate, amortized allocation overhead should be zero. {0, `Fprintf(buf, "%x %x %x")`, func() { mallocBuf.Reset() diff --git a/src/fmt/print.go b/src/fmt/print.go index 85f70439f3..8082d13874 100644 --- a/src/fmt/print.go +++ b/src/fmt/print.go @@ -172,15 +172,17 @@ func newPrinter() *pp { func (p *pp) free() { // Proper usage of a sync.Pool requires each entry to have approximately // the same memory cost. To obtain this property when the stored type - // contains a variably-sized buffer, we add a hard limit on the maximum buffer - // to place back in the pool. + // contains a variably-sized buffer, we add a hard limit on the maximum + // buffer to place back in the pool. If the buffer is larger than the + // limit, we drop the buffer and recycle just the printer. // - // See https://golang.org/issue/23199 - if cap(p.buf) > 64<<10 { - return + // See https://golang.org/issue/23199. + if cap(p.buf) > 64*1024 { + p.buf = nil + } else { + p.buf = p.buf[:0] } - p.buf = p.buf[:0] p.arg = nil p.value = reflect.Value{} p.wrappedErr = nil