Skip to content

Commit 3a3eaaf

Browse files
perf: Optimize Stringify allocations (~3x faster) (#3914)
1 parent 2790da5 commit 3a3eaaf

File tree

3 files changed

+171
-3
lines changed

3 files changed

+171
-3
lines changed

github/strings.go

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,30 @@ import (
99
"bytes"
1010
"fmt"
1111
"reflect"
12+
"strconv"
13+
"sync"
1214
)
1315

1416
var timestampType = reflect.TypeFor[Timestamp]()
1517

18+
var bufferPool = sync.Pool{
19+
New: func() any {
20+
return new(bytes.Buffer)
21+
},
22+
}
23+
1624
// Stringify attempts to create a reasonable string representation of types in
1725
// the GitHub library. It does things like resolve pointers to their values
1826
// and omits struct fields with nil values.
1927
func Stringify(message any) string {
20-
var buf bytes.Buffer
28+
buf := bufferPool.Get().(*bytes.Buffer)
29+
defer func() {
30+
buf.Reset()
31+
bufferPool.Put(buf)
32+
}()
33+
2134
v := reflect.ValueOf(message)
22-
stringifyValue(&buf, v)
35+
stringifyValue(buf, v)
2336
return buf.String()
2437
}
2538

@@ -34,8 +47,20 @@ func stringifyValue(w *bytes.Buffer, val reflect.Value) {
3447
v := reflect.Indirect(val)
3548

3649
switch v.Kind() {
50+
case reflect.Bool:
51+
w.Write(strconv.AppendBool(w.Bytes(), v.Bool())[w.Len():])
52+
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
53+
w.Write(strconv.AppendInt(w.Bytes(), v.Int(), 10)[w.Len():])
54+
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
55+
w.Write(strconv.AppendUint(w.Bytes(), v.Uint(), 10)[w.Len():])
56+
case reflect.Float32:
57+
w.Write(strconv.AppendFloat(w.Bytes(), v.Float(), 'g', -1, 32)[w.Len():])
58+
case reflect.Float64:
59+
w.Write(strconv.AppendFloat(w.Bytes(), v.Float(), 'g', -1, 64)[w.Len():])
3760
case reflect.String:
38-
fmt.Fprintf(w, `"%v"`, v)
61+
w.WriteByte('"')
62+
w.WriteString(v.String())
63+
w.WriteByte('"')
3964
case reflect.Slice:
4065
w.WriteByte('[')
4166
for i := range v.Len() {

github/strings_benchmark_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2026 The go-github AUTHORS. All rights reserved.
2+
//
3+
// Use of this source code is governed by a BSD-style
4+
// license that can be found in the LICENSE file.
5+
6+
package github
7+
8+
import (
9+
"testing"
10+
)
11+
12+
type BenchmarkStruct struct {
13+
Name string
14+
Age int
15+
Active bool
16+
Score float32
17+
Rank float64
18+
Tags []string
19+
Pointer *int
20+
}
21+
22+
func BenchmarkStringify(b *testing.B) {
23+
val := 42
24+
s := &BenchmarkStruct{
25+
Name: "benchmark",
26+
Age: 30,
27+
Active: true,
28+
Score: 1.1,
29+
Rank: 99.999999,
30+
Tags: []string{"go", "github", "api"},
31+
Pointer: Ptr(val),
32+
}
33+
b.ResetTimer()
34+
for b.Loop() {
35+
Stringify(s)
36+
}
37+
}

github/strings_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,92 @@ func TestStringify(t *testing.T) {
8181
}
8282
}
8383

84+
func TestStringify_Primitives(t *testing.T) {
85+
t.Parallel()
86+
tests := []struct {
87+
in any
88+
out string
89+
}{
90+
// Bool
91+
{true, "true"},
92+
{false, "false"},
93+
94+
// Int variants
95+
{int(1), "1"},
96+
{int8(2), "2"},
97+
{int16(3), "3"},
98+
{int32(4), "4"},
99+
{int64(5), "5"},
100+
101+
// Uint variants
102+
{uint(6), "6"},
103+
{uint8(7), "7"},
104+
{uint16(8), "8"},
105+
{uint32(9), "9"},
106+
{uint64(10), "10"},
107+
{uintptr(11), "11"},
108+
109+
// Float variants (Precision Correctness)
110+
{float32(1.1), "1.1"},
111+
{float64(1.1), "1.1"},
112+
{float32(1.0000001), "1.0000001"},
113+
{float64(1.000000000000001), "1.000000000000001"},
114+
115+
// Boundary Cases
116+
{int8(-128), "-128"},
117+
{int8(127), "127"},
118+
{uint64(18446744073709551615), "18446744073709551615"},
119+
120+
// String Optimization
121+
{"hello", `"hello"`},
122+
{"", `""`},
123+
}
124+
125+
for i, tt := range tests {
126+
s := Stringify(tt.in)
127+
if s != tt.out {
128+
t.Errorf("%v. Stringify(%T) => %q, want %q", i, tt.in, s, tt.out)
129+
}
130+
}
131+
}
132+
133+
func TestStringify_BufferPool(t *testing.T) {
134+
t.Parallel()
135+
// Verify that concurrent usage of Stringify is safe and doesn't corrupt buffers.
136+
// While we can't easily verify reuse without exposing internal metrics,
137+
// we can verify correctness under load which implies proper Reset() handling.
138+
const goroutines = 10
139+
const iterations = 100
140+
141+
errCh := make(chan error, goroutines)
142+
143+
for range goroutines {
144+
go func() {
145+
for range iterations {
146+
// Use a mix of types to exercise different code paths
147+
s1 := Stringify(123)
148+
if s1 != "123" {
149+
errCh <- fmt.Errorf("got %q, want %q", s1, "123")
150+
return
151+
}
152+
153+
s2 := Stringify("test")
154+
if s2 != `"test"` {
155+
errCh <- fmt.Errorf("got %q, want %q", s2, `"test"`)
156+
return
157+
}
158+
}
159+
errCh <- nil
160+
}()
161+
}
162+
163+
for range goroutines {
164+
if err := <-errCh; err != nil {
165+
t.Error(err)
166+
}
167+
}
168+
}
169+
84170
// Directly test the String() methods on various GitHub types. We don't do an
85171
// exhaustive test of all the various field types, since TestStringify() above
86172
// takes care of that. Rather, we just make sure that Stringify() is being
@@ -143,3 +229,23 @@ func TestString(t *testing.T) {
143229
}
144230
}
145231
}
232+
233+
func TestStringify_Floats(t *testing.T) {
234+
t.Parallel()
235+
tests := []struct {
236+
in any
237+
out string
238+
}{
239+
{float32(1.1), "1.1"},
240+
{float64(1.1), "1.1"},
241+
{float32(1.0000001), "1.0000001"},
242+
{struct{ F float32 }{1.1}, "{F:1.1}"},
243+
}
244+
245+
for i, tt := range tests {
246+
s := Stringify(tt.in)
247+
if s != tt.out {
248+
t.Errorf("%v. Stringify(%v) = %q, want %q", i, tt.in, s, tt.out)
249+
}
250+
}
251+
}

0 commit comments

Comments
 (0)