Skip to content

Commit 425f6de

Browse files
committed
jsontext: fix Decoder.Reset to preserve internal buffer capacity
The Decoder.Reset method is not preserving the internal buffer between resets, causing buffer capacity to be lost and resulting in unnecessary allocations when reusing decoders. This is particularly problematic when decoding many small messages. This commit fixes the Reset method to preserve the internal buffer. It makes sure aliasing is removed if the buffer currently points to an internal byte slice of a bytes.Buffer. It adds a TestDecoderReset test structured into subtests to better validate the different scenarios.
1 parent 7fa2c73 commit 425f6de

File tree

2 files changed

+93
-1
lines changed

2 files changed

+93
-1
lines changed

src/encoding/json/jsontext/decode.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ type decodeBuffer struct {
103103
peekPos int // non-zero if valid offset into buf for start of next token
104104
peekErr error // implies peekPos is -1
105105

106+
bufAlias bool // true if buf aliases rd
106107
buf []byte // may alias rd if it is a bytes.Buffer
107108
prevStart int
108109
prevEnd int
@@ -138,7 +139,14 @@ func (d *Decoder) Reset(r io.Reader, opts ...Options) {
138139
case d.s.Flags.Get(jsonflags.WithinArshalCall):
139140
panic("jsontext: cannot reset Decoder passed to json.UnmarshalerFrom")
140141
}
141-
d.s.reset(nil, r, opts...)
142+
// If the decoder was previously aliasing a bytes.Buffer,
143+
// invalidate the alias to avoid writing into the bytes.Buffer's
144+
// internal buffer.
145+
if d.s.bufAlias {
146+
d.s.buf = nil
147+
d.s.bufAlias = false
148+
}
149+
d.s.reset(d.s.buf[:0], r, opts...)
142150
}
143151

144152
func (d *decoderState) reset(b []byte, r io.Reader, opts ...Options) {
@@ -179,6 +187,7 @@ func (d *decoderState) fetch() error {
179187
case bb.Len() == 0:
180188
return io.ErrUnexpectedEOF
181189
case len(d.buf) == 0:
190+
d.bufAlias = true
182191
d.buf = bb.Next(bb.Len()) // "read" all data in the buffer
183192
return nil
184193
default:

src/encoding/json/jsontext/decode_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,3 +1265,86 @@ func TestPeekableDecoder(t *testing.T) {
12651265
}
12661266
}
12671267
}
1268+
1269+
// TestDecoderReset tests that the decoder preserves its internal
1270+
// buffer between Reset calls to avoid frequent allocations when reusing the decoder.
1271+
// It ensures that the buffer capacity is maintained while avoiding aliasing
1272+
// issues with bytes.Buffer.
1273+
func TestDecoderReset(t *testing.T) {
1274+
// Create a decoder with a reasonably large JSON input to ensure buffer growth
1275+
largeJSON := `{"key1":"value1","key2":"value2","key3":"value3","key4":"value4","key5":"value5"}`
1276+
dec := NewDecoder(strings.NewReader(largeJSON))
1277+
1278+
t.Run("Test capacity preservation", func(t *testing.T) {
1279+
// Read the first JSON value to grow the internal buffer
1280+
val1, err := dec.ReadValue()
1281+
if err != nil {
1282+
t.Fatalf("first ReadValue failed: %v", err)
1283+
}
1284+
if string(val1) != largeJSON {
1285+
t.Fatalf("first ReadValue = %q, want %q", val1, largeJSON)
1286+
}
1287+
1288+
// Get the buffer capacity after first use
1289+
initialCapacity := cap(dec.s.buf)
1290+
if initialCapacity == 0 {
1291+
t.Fatalf("expected non-zero buffer capacity after first use")
1292+
}
1293+
1294+
// Reset with a new reader - this should preserve the buffer capacity
1295+
dec.Reset(strings.NewReader(largeJSON))
1296+
1297+
// Verify the buffer capacity is preserved (or at least not smaller)
1298+
preservedCapacity := cap(dec.s.buf)
1299+
if preservedCapacity < initialCapacity {
1300+
t.Fatalf("buffer capacity reduced after Reset: got %d, want at least %d", preservedCapacity, initialCapacity)
1301+
}
1302+
1303+
// Read the second JSON value to ensure the decoder still works correctly
1304+
val2, err := dec.ReadValue()
1305+
if err != nil {
1306+
t.Fatalf("second ReadValue failed: %v", err)
1307+
}
1308+
if string(val2) != largeJSON {
1309+
t.Fatalf("second ReadValue = %q, want %q", val2, largeJSON)
1310+
}
1311+
})
1312+
1313+
var bbBuf []byte
1314+
t.Run("Test aliasing with bytes.Buffer", func(t *testing.T) {
1315+
// Test with bytes.Buffer to verify proper aliasing behavior.
1316+
bb := bytes.NewBufferString(largeJSON)
1317+
dec.Reset(bb)
1318+
bbBuf = bb.Bytes()
1319+
1320+
// Read the third JSON value to ensure functionality with bytes.Buffer
1321+
val3, err := dec.ReadValue()
1322+
if err != nil {
1323+
t.Fatalf("fourth ReadValue failed: %v", err)
1324+
}
1325+
if string(val3) != largeJSON {
1326+
t.Fatalf("fourth ReadValue = %q, want %q", val3, largeJSON)
1327+
}
1328+
// The decoder buffer should alias bytes.Buffer's internal buffer.
1329+
if len(dec.s.buf) == 0 || len(bbBuf) == 0 || &dec.s.buf[0] != &bbBuf[0] {
1330+
t.Fatalf("decoder buffer does not alias bytes.Buffer")
1331+
}
1332+
})
1333+
1334+
t.Run("Test aliasing removed after Reset", func(t *testing.T) {
1335+
// Reset with a new reader and verify the buffer is not aliased.
1336+
dec.Reset(strings.NewReader(largeJSON))
1337+
val4, err := dec.ReadValue()
1338+
if err != nil {
1339+
t.Fatalf("fifth ReadValue failed: %v", err)
1340+
}
1341+
if string(val4) != largeJSON {
1342+
t.Fatalf("fourth ReadValue = %q, want %q", val4, largeJSON)
1343+
}
1344+
1345+
// The decoder buffer should not alias the bytes.Buffer's internal buffer.
1346+
if len(dec.s.buf) == 0 || len(bbBuf) == 0 || &dec.s.buf[0] == &bbBuf[0] {
1347+
t.Fatalf("decoder buffer aliases bytes.Buffer")
1348+
}
1349+
})
1350+
}

0 commit comments

Comments
 (0)