perf: hoist newValue allocations out of typed map decode loop (#65)

CHANGELOG.md

@@ -6,6 +6,7 @@
 - **decode:** `PeekCode` bsr fast path — peeks directly at `bsr.data[bsr.pos]` instead of `ReadByte` + `UnreadByte` (two interface calls) ([#59](https://github.com/Basekick-Labs/msgpack/issues/59))
 - **encode:** pool `OmitEmpty` filtered field slices via `sync.Pool` — when fields are actually omitted, the allocated `[]*field` slice is now returned to a pool for reuse instead of being GC'd ([#58](https://github.com/Basekick-Labs/msgpack/issues/58))
 - **encode/decode:** pool and pre-allocate interned-string dict — `SetInternedStringsDictCap(n)` pre-sizes the dict to avoid map rehashing and slice growth; pooled encoders/decoders now reuse dict storage across `Reset()` (cleared in place) instead of discarding it, and `Put*()` drops oversized dicts to keep the pool lean ([#66](https://github.com/Basekick-Labs/msgpack/issues/66))
+- **decode:** hoist `newValue()` allocations out of `decodeTypedMapValue` loop — reuses a single key slot and value slot across all map entries, zeroing between iterations. Takes typed-map decode from 2N `reflect.New()` calls to 2 per map ([#65](https://github.com/Basekick-Labs/msgpack/issues/65)) (BenchmarkLargeMapIntInt **-50% allocs/op**, **-50% B/op**, **-10% ns/op** for 1000-entry `map[int]int`)
 
 ---
 

bench_test.go

@@ -192,6 +192,15 @@ func BenchmarkMapIntInt(b *testing.B) {
 	benchmarkEncodeDecode(b, src, &dst)
 }
 
+func BenchmarkLargeMapIntInt(b *testing.B) {
+	src := make(map[int]int, 1000)
+	for i := 0; i < 1000; i++ {
+		src[i] = i
+	}
+	var dst map[int]int
+	benchmarkEncodeDecode(b, src, &dst)
+}
+
 func BenchmarkStringSlice(b *testing.B) {
 	src := []string{"hello", "world"}
 	var dst []string

decode_map.go

@@ -291,18 +291,34 @@ func (d *Decoder) decodeTypedMapN(n int) (interface{}, error) {
 }
 
 func (d *Decoder) decodeTypedMapValue(v reflect.Value, n int) error {
+	if n == 0 {
+		return nil
+	}
 	var (
 		typ       = v.Type()
 		keyType   = typ.Key()
 		valueType = typ.Elem()
 	)
+	// Hoist the key and value slots out of the loop so we pay two
+	// reflect.New allocations per map decode instead of 2N. SetMapIndex
+	// copies the key and value, so the stored entries are unaffected by
+	// subsequent mutation of mk/mv.
+	//
+	// Zeroing each iteration is required for correctness, not just
+	// hygiene: value decoders like decodeSliceValue reuse an existing
+	// slice's backing array when v.Cap() >= n, which would otherwise let
+	// iteration 2 clobber iteration 1's already-stored slice.
+	mk := d.newValue(keyType).Elem()
+	mv := d.newValue(valueType).Elem()
+	keyZero := reflect.Zero(keyType)
+	valueZero := reflect.Zero(valueType)
 	for i := 0; i < n; i++ {
-		mk := d.newValue(keyType).Elem()
+		mk.Set(keyZero)
 		if err := d.DecodeValue(mk); err != nil {
 			return err
 		}
 
-		mv := d.newValue(valueType).Elem()
+		mv.Set(valueZero)
 		if err := d.DecodeValue(mv); err != nil {
 			return err
 		}

msgpack_test.go

@@ -553,3 +553,65 @@ func TestPtrValueDecode(t *testing.T) {
 	require.NotNil(t, foo.Bar)
 	require.Equal(t, *foo.Bar, bar2)
 }
+
+// TestDecodeTypedMapSliceValuesAreIndependent is the regression test for #65.
+// decodeTypedMapValue hoists the key/value reflect.Value slots out of the
+// per-entry loop and zeroes them each iteration. Without the zeroing step,
+// decodeSliceValue would reuse the previous iteration's backing array
+// (since it reuses capacity when v.Cap() >= n), corrupting entries already
+// written to the map.
+func TestDecodeTypedMapSliceValuesAreIndependent(t *testing.T) {
+	in := map[int][]int{
+		1: {10, 11, 12},
+		2: {20, 21, 22},
+		3: {30, 31, 32},
+	}
+
+	b, err := msgpack.Marshal(in)
+	require.Nil(t, err)
+
+	var out map[int][]int
+	require.Nil(t, msgpack.Unmarshal(b, &out))
+	require.Equal(t, in, out)
+
+	// Mutating one entry must not touch any other. If the decode loop
+	// reused a single backing array, the entries would alias and this
+	// assertion would fail.
+	for i := range out[1] {
+		out[1][i] = -1
+	}
+	require.Equal(t, []int{20, 21, 22}, out[2])
+	require.Equal(t, []int{30, 31, 32}, out[3])
+}
+
+// TestDecodeTypedMapStructValuesAreIndependent covers the struct-value
+// case of the same hoist bug. The struct carries a slice field so the
+// aliasing mechanism fires through the hoisted value slot: decodeStructValue
+// writes every field present in the payload, but the slice field itself is
+// decoded via decodeSliceValue, which reuses the existing backing array
+// when capacity is sufficient. Without per-iteration zeroing of the outer
+// struct's slot, iteration 2's slice field would overwrite iteration 1's.
+func TestDecodeTypedMapStructValuesAreIndependent(t *testing.T) {
+	type kv struct {
+		A int
+		B []string
+	}
+	in := map[int]kv{
+		1: {1, []string{"one", "uno"}},
+		2: {2, []string{"two", "dos"}},
+		3: {3, []string{"three", "tres"}},
+	}
+
+	b, err := msgpack.Marshal(in)
+	require.Nil(t, err)
+
+	var out map[int]kv
+	require.Nil(t, msgpack.Unmarshal(b, &out))
+	require.Equal(t, in, out)
+
+	for i := range out[1].B {
+		out[1].B[i] = "MUTATED"
+	}
+	require.Equal(t, []string{"two", "dos"}, out[2].B)
+	require.Equal(t, []string{"three", "tres"}, out[3].B)
+}

types_test.go

@@ -548,6 +548,14 @@ var (
 		{in: mapStringInterface{"foo": "bar"}, out: new(mapStringInterface)},
 		{in: map[stringAlias]interfaceAlias{"foo": "bar"}, out: new(map[stringAlias]interfaceAlias)},
 		{in: map[int]string{1: "string"}, out: new(map[int]string)},
+		// Map value types whose decoder reuses existing state (slice
+		// backing array, nested map, struct fields). Regression coverage
+		// for #65: decodeTypedMapValue hoists key/value slots out of the
+		// loop, so these types would clobber earlier entries if the
+		// per-iteration zeroing were removed.
+		{in: map[int][]int{1: {1, 2, 3}, 2: {4, 5, 6}, 3: {7, 8, 9}}, out: new(map[int][]int)},
+		{in: map[string][]string{"a": {"x", "y"}, "b": {"z"}}, out: new(map[string][]string)},
+		{in: map[string]map[string]int{"o1": {"i": 1}, "o2": {"i": 2}}, out: new(map[string]map[string]int)},
 
 		{in: (*Object)(nil), out: new(*Object)},
 		{in: &Object{42}, out: new(Object)},