Skip to content

Commit 1325a05

Browse files
cherrymuidsnet
authored andcommitted
proto: fix quadratic behavior in nested map marshaling (#641)
If the value of a map is a message with nested maps, calling valSizer in marshal may be quadratic -- it will call size at each level. Fix by using cached size if the value type is message. Benchmark results with @LMMilewski's test case from #624: name old time/op new time/op delta 1-12 1.96µs ±19% 1.76µs ±18% ~ (p=0.075 n=10+10) 2-12 5.10µs ± 6% 3.84µs ± 8% -24.69% (p=0.000 n=9+10) 4-12 14.2µs ±12% 8.2µs ±15% -42.77% (p=0.000 n=10+10) 8-12 43.0µs ±10% 15.6µs ±11% -63.81% (p=0.000 n=9+10) 16-12 138µs ±12% 33µs ±17% -76.32% (p=0.000 n=10+10) name old alloc/op new alloc/op delta 1-12 376B ± 0% 376B ± 0% ~ (all equal) 2-12 928B ± 0% 752B ± 0% -18.97% (p=0.000 n=10+10) 4-12 2.54kB ± 0% 1.49kB ± 0% -41.51% (p=0.000 n=10+10) 8-12 7.89kB ± 0% 2.96kB ± 0% -62.47% (p=0.000 n=10+10) 16-12 27.0kB ± 0% 5.9kB ± 0% -78.11% (p=0.000 n=10+10) name old allocs/op new allocs/op delta 1-12 12.0 ± 0% 12.0 ± 0% ~ (all equal) 2-12 28.0 ± 0% 23.0 ± 0% -17.86% (p=0.000 n=10+10) 4-12 75.0 ± 0% 45.0 ± 0% -40.00% (p=0.000 n=10+10) 8-12 229 ± 0% 89 ± 0% -61.14% (p=0.000 n=10+10) 16-12 777 ± 0% 177 ± 0% -77.22% (p=0.000 n=10+10) Fixes #624.
1 parent f05648d commit 1325a05

File tree

1 file changed

+20
-1
lines changed

1 file changed

+20
-1
lines changed

proto/table_marshal.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2278,6 +2278,25 @@ func makeMapMarshaler(f *reflect.StructField) (sizer, marshaler) {
22782278
// value.
22792279
// Key cannot be pointer-typed.
22802280
valIsPtr := valType.Kind() == reflect.Ptr
2281+
2282+
// If value is a message with nested maps, calling
2283+
// valSizer in marshal may be quadratic. We should use
2284+
// cached version in marshal (but not in size).
2285+
// If value is not message type, we don't have size cache,
2286+
// but it cannot be nested either. Just use valSizer.
2287+
valCachedSizer := valSizer
2288+
if valIsPtr && valType.Elem().Kind() == reflect.Struct {
2289+
u := getMarshalInfo(valType.Elem())
2290+
valCachedSizer = func(ptr pointer, tagsize int) int {
2291+
// Same as message sizer, but use cache.
2292+
p := ptr.getPointer()
2293+
if p.isNil() {
2294+
return 0
2295+
}
2296+
siz := u.cachedsize(p)
2297+
return siz + SizeVarint(uint64(siz)) + tagsize
2298+
}
2299+
}
22812300
return func(ptr pointer, tagsize int) int {
22822301
m := ptr.asPointerTo(t).Elem() // the map
22832302
n := 0
@@ -2304,7 +2323,7 @@ func makeMapMarshaler(f *reflect.StructField) (sizer, marshaler) {
23042323
kaddr := toAddrPointer(&ki, false) // pointer to key
23052324
vaddr := toAddrPointer(&vi, valIsPtr) // pointer to value
23062325
b = appendVarint(b, tag)
2307-
siz := keySizer(kaddr, 1) + valSizer(vaddr, 1) // tag of key = 1 (size=1), tag of val = 2 (size=1)
2326+
siz := keySizer(kaddr, 1) + valCachedSizer(vaddr, 1) // tag of key = 1 (size=1), tag of val = 2 (size=1)
23082327
b = appendVarint(b, uint64(siz))
23092328
b, err = keyMarshaler(b, kaddr, keyWireTag, deterministic)
23102329
if err != nil {

0 commit comments

Comments
 (0)