Skip to content

Commit f798dc6

Browse files
CAFxXmvdan
authored andcommitted
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 <[email protected]> Run-TryBot: Rob Pike <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> Reviewed-by: Rob Pike <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 9fc09d2 commit f798dc6

File tree

2 files changed

+18
-11
lines changed

2 files changed

+18
-11
lines changed

src/fmt/fmt_test.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -1429,11 +1429,16 @@ var mallocTest = []struct {
14291429
}{
14301430
{0, `Sprintf("")`, func() { Sprintf("") }},
14311431
{1, `Sprintf("xxx")`, func() { Sprintf("xxx") }},
1432-
{2, `Sprintf("%x")`, func() { Sprintf("%x", 7) }},
1433-
{2, `Sprintf("%s")`, func() { Sprintf("%s", "hello") }},
1434-
{3, `Sprintf("%x %x")`, func() { Sprintf("%x %x", 7, 112) }},
1435-
{2, `Sprintf("%g")`, func() { Sprintf("%g", float32(3.14159)) }}, // TODO: Can this be 1?
1436-
{1, `Fprintf(buf, "%s")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%s", "hello") }},
1432+
{0, `Sprintf("%x")`, func() { Sprintf("%x", 7) }},
1433+
{1, `Sprintf("%x")`, func() { Sprintf("%x", 1<<16) }},
1434+
{3, `Sprintf("%80000s")`, func() { Sprintf("%80000s", "hello") }}, // large buffer (>64KB)
1435+
{1, `Sprintf("%s")`, func() { Sprintf("%s", "hello") }},
1436+
{1, `Sprintf("%x %x")`, func() { Sprintf("%x %x", 7, 112) }},
1437+
{1, `Sprintf("%g")`, func() { Sprintf("%g", float32(3.14159)) }},
1438+
{0, `Fprintf(buf, "%s")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%s", "hello") }},
1439+
{0, `Fprintf(buf, "%x")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%x", 7) }},
1440+
{0, `Fprintf(buf, "%x")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%x", 1<<16) }},
1441+
{2, `Fprintf(buf, "%80000s")`, func() { mallocBuf.Reset(); Fprintf(&mallocBuf, "%80000s", "hello") }}, // large buffer (>64KB)
14371442
// If the interface value doesn't need to allocate, amortized allocation overhead should be zero.
14381443
{0, `Fprintf(buf, "%x %x %x")`, func() {
14391444
mallocBuf.Reset()

src/fmt/print.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -172,15 +172,17 @@ func newPrinter() *pp {
172172
func (p *pp) free() {
173173
// Proper usage of a sync.Pool requires each entry to have approximately
174174
// the same memory cost. To obtain this property when the stored type
175-
// contains a variably-sized buffer, we add a hard limit on the maximum buffer
176-
// to place back in the pool.
175+
// contains a variably-sized buffer, we add a hard limit on the maximum
176+
// buffer to place back in the pool. If the buffer is larger than the
177+
// limit, we drop the buffer and recycle just the printer.
177178
//
178-
// See https://golang.org/issue/23199
179-
if cap(p.buf) > 64<<10 {
180-
return
179+
// See https://golang.org/issue/23199.
180+
if cap(p.buf) > 64*1024 {
181+
p.buf = nil
182+
} else {
183+
p.buf = p.buf[:0]
181184
}
182185

183-
p.buf = p.buf[:0]
184186
p.arg = nil
185187
p.value = reflect.Value{}
186188
p.wrappedErr = nil

0 commit comments

Comments
 (0)