From 59c8421597854a99840027e73b962d48e601292b Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 26 Jul 2023 16:53:20 +0300 Subject: [PATCH 1/6] [#49] util/proto: Use StableSize() to determine if the struct is empty `reflect` is not necessary here, and checking `StableSize` is what we _want_ anyway. Signed-off-by: Evgenii Stratonikov --- status/marshal.go | 4 ++++ util/proto/marshal.go | 10 ++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/status/marshal.go b/status/marshal.go index 78064c1..1868a43 100644 --- a/status/marshal.go +++ b/status/marshal.go @@ -69,6 +69,10 @@ func (x *Status) StableMarshal(buf []byte) []byte { } func (x *Status) StableSize() (size int) { + if x == nil { + return 0 + } + size += protoutil.UInt32Size(statusCodeFNum, CodeToGRPC(x.code)) size += protoutil.StringSize(statusMsgFNum, x.msg) diff --git a/util/proto/marshal.go b/util/proto/marshal.go index a82478b..ff0d30b 100644 --- a/util/proto/marshal.go +++ b/util/proto/marshal.go @@ -10,7 +10,6 @@ import ( "encoding/binary" "math" "math/bits" - "reflect" ) type ( @@ -303,14 +302,13 @@ func NestedStructurePrefix(field int64) (prefix uint64, ln int) { } func NestedStructureMarshal(field int64, buf []byte, v stableMarshaller) int { - if v == nil || reflect.ValueOf(v).IsNil() { + n := v.StableSize() + if n == 0 { return 0 } prefix, _ := NestedStructurePrefix(field) offset := binary.PutUvarint(buf, prefix) - - n := v.StableSize() offset += binary.PutUvarint(buf[offset:], uint64(n)) v.StableMarshal(buf[offset:]) @@ -318,12 +316,12 @@ func NestedStructureMarshal(field int64, buf []byte, v stableMarshaller) int { } func NestedStructureSize(field int64, v stableMarshaller) (size int) { - if v == nil || reflect.ValueOf(v).IsNil() { + n := v.StableSize() + if n == 0 { return 0 } _, ln := NestedStructurePrefix(field) - n := v.StableSize() size = ln + VarUIntSize(uint64(n)) + n return size -- 2.45.3 From 849de02bc3968ec69fda328d6a7c673bbbbd0928 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 26 Jul 2023 17:52:54 +0300 Subject: [PATCH 2/6] [#49] util/proto: Calculate repeated field size without allocations Signed-off-by: Evgenii Stratonikov --- util/proto/marshal.go | 100 ++++++++++++------------------------------ 1 file changed, 27 insertions(+), 73 deletions(-) diff --git a/util/proto/marshal.go b/util/proto/marshal.go index ff0d30b..7bc8162 100644 --- a/util/proto/marshal.go +++ b/util/proto/marshal.go @@ -178,30 +178,13 @@ func RepeatedStringSize(field int, v []string) (size int) { return size } -func RepeatedUInt64Marshal(field int, buf []byte, v []uint64) int { - if len(v) == 0 { - return 0 - } - - prefix := field<<3 | 0x02 - offset := binary.PutUvarint(buf, uint64(prefix)) - - _, arrSize := RepeatedUInt64Size(field, v) - offset += binary.PutUvarint(buf[offset:], uint64(arrSize)) - for i := range v { - offset += binary.PutUvarint(buf[offset:], v[i]) - } - - return offset -} - -func RepeatedUInt64Size(field int, v []uint64) (size, arraySize int) { +func repeatedUIntSize[T ~uint64 | ~int64 | ~uint32 | ~int32](field int, v []T) (size, arraySize int) { if len(v) == 0 { return 0, 0 } for i := range v { - size += VarUIntSize(v[i]) + size += VarUIntSize(uint64(v[i])) } arraySize = size @@ -213,82 +196,53 @@ func RepeatedUInt64Size(field int, v []uint64) (size, arraySize int) { return size, arraySize } -func RepeatedInt64Marshal(field int, buf []byte, v []int64) int { +func repeatedUIntMarshal[T ~uint64 | ~int64 | ~uint32 | ~int32](field int, buf []byte, v []T) int { if len(v) == 0 { return 0 } - convert := make([]uint64, len(v)) + prefix := field<<3 | 0x02 + offset := binary.PutUvarint(buf, uint64(prefix)) + + _, arrSize := repeatedUIntSize(field, v) + offset += binary.PutUvarint(buf[offset:], uint64(arrSize)) for i := range v { - convert[i] = uint64(v[i]) + offset += binary.PutUvarint(buf[offset:], uint64(v[i])) } - return RepeatedUInt64Marshal(field, buf, convert) + return offset +} + +func RepeatedUInt64Marshal(field int, buf []byte, v []uint64) int { + return repeatedUIntMarshal(field, buf, v) +} + +func RepeatedUInt64Size(field int, v []uint64) (size, arraySize int) { + return repeatedUIntSize(field, v) +} + +func RepeatedInt64Marshal(field int, buf []byte, v []int64) int { + return repeatedUIntMarshal(field, buf, v) } func RepeatedInt64Size(field int, v []int64) (size, arraySize int) { - if len(v) == 0 { - return 0, 0 - } - - convert := make([]uint64, len(v)) - for i := range v { - convert[i] = uint64(v[i]) - } - - return RepeatedUInt64Size(field, convert) + return repeatedUIntSize(field, v) } func RepeatedUInt32Marshal(field int, buf []byte, v []uint32) int { - if len(v) == 0 { - return 0 - } - - convert := make([]uint64, len(v)) - for i := range v { - convert[i] = uint64(v[i]) - } - - return RepeatedUInt64Marshal(field, buf, convert) + return repeatedUIntMarshal(field, buf, v) } func RepeatedUInt32Size(field int, v []uint32) (size, arraySize int) { - if len(v) == 0 { - return 0, 0 - } - - convert := make([]uint64, len(v)) - for i := range v { - convert[i] = uint64(v[i]) - } - - return RepeatedUInt64Size(field, convert) + return repeatedUIntSize(field, v) } func RepeatedInt32Marshal(field int, buf []byte, v []int32) int { - if len(v) == 0 { - return 0 - } - - convert := make([]uint64, len(v)) - for i := range v { - convert[i] = uint64(v[i]) - } - - return RepeatedUInt64Marshal(field, buf, convert) + return repeatedUIntMarshal(field, buf, v) } func RepeatedInt32Size(field int, v []int32) (size, arraySize int) { - if len(v) == 0 { - return 0, 0 - } - - convert := make([]uint64, len(v)) - for i := range v { - convert[i] = uint64(v[i]) - } - - return RepeatedUInt64Size(field, convert) + return repeatedUIntSize(field, v) } // VarUIntSize returns length of varint byte sequence for uint64 value 'x'. -- 2.45.3 From 6e92d7d5de6ba6b346b0c8d27c1801b5a0c73580 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 26 Jul 2023 18:12:27 +0300 Subject: [PATCH 3/6] [#49] util/proto: Make `NestedStructure*` generic Signed-off-by: Evgenii Stratonikov --- util/proto/marshal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/proto/marshal.go b/util/proto/marshal.go index 7bc8162..e345492 100644 --- a/util/proto/marshal.go +++ b/util/proto/marshal.go @@ -255,7 +255,7 @@ func NestedStructurePrefix(field int64) (prefix uint64, ln int) { return prefix, VarUIntSize(prefix) } -func NestedStructureMarshal(field int64, buf []byte, v stableMarshaller) int { +func NestedStructureMarshal[T stableMarshaller](field int64, buf []byte, v T) int { n := v.StableSize() if n == 0 { return 0 @@ -269,7 +269,7 @@ func NestedStructureMarshal(field int64, buf []byte, v stableMarshaller) int { return offset + n } -func NestedStructureSize(field int64, v stableMarshaller) (size int) { +func NestedStructureSize[T stableMarshaller](field int64, v T) (size int) { n := v.StableSize() if n == 0 { return 0 -- 2.45.3 From 43ad0f114cf5ce5bd516b933ed08f721d27741a8 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 26 Jul 2023 18:12:48 +0300 Subject: [PATCH 4/6] [#49] session: Make StableSize() zero-alloc For this to work, it is necessary that `NestedStructureSize` is a generic function. Otherwise, we would allocate to put it in the interface. Signed-off-by: Evgenii Stratonikov --- session/marshal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/session/marshal.go b/session/marshal.go index 3c56cd4..cda9579 100644 --- a/session/marshal.go +++ b/session/marshal.go @@ -211,7 +211,7 @@ func (c *ObjectSessionContext) StableMarshal(buf []byte) []byte { } offset := proto.EnumMarshal(objectCtxVerbField, buf, int32(c.verb)) - proto.NestedStructureMarshal(objectCtxTargetField, buf[offset:], &objectSessionContextTarget{ + proto.NestedStructureMarshal(objectCtxTargetField, buf[offset:], objectSessionContextTarget{ cnr: c.cnr, objs: c.objs, }) @@ -225,7 +225,7 @@ func (c *ObjectSessionContext) StableSize() (size int) { } size += proto.EnumSize(objectCtxVerbField, int32(c.verb)) - size += proto.NestedStructureSize(objectCtxTargetField, &objectSessionContextTarget{ + size += proto.NestedStructureSize(objectCtxTargetField, objectSessionContextTarget{ cnr: c.cnr, objs: c.objs, }) -- 2.45.3 From 7133a01ccf50844b39414e804d18a10b6c890a90 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 26 Jul 2023 18:13:05 +0300 Subject: [PATCH 5/6] [#49] message/test: Add test for zero-alloc StableSize() Signed-off-by: Evgenii Stratonikov --- rpc/message/test/message.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rpc/message/test/message.go b/rpc/message/test/message.go index df769d2..435f20a 100644 --- a/rpc/message/test/message.go +++ b/rpc/message/test/message.go @@ -17,6 +17,7 @@ type jsonMessage interface { type binaryMessage interface { StableMarshal([]byte) []byte + StableSize() int Unmarshal([]byte) error } @@ -53,6 +54,11 @@ func TestRPCMessage(t *testing.T, msgGens ...func(empty bool) message.Message) { } if bm, ok := msg.(binaryMessage); ok { + t.Run(fmt.Sprintf("%T.StableSize() does no allocations", bm), func(t *testing.T) { + require.Zero(t, testing.AllocsPerRun(1000, func() { + _ = bm.StableSize() + })) + }) t.Run(fmt.Sprintf("Binary_%T", msg), func(t *testing.T) { data := bm.StableMarshal(nil) -- 2.45.3 From 7a5ee927c8a272ff97f26cd8b7aa65a4c9d6d161 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 26 Jul 2023 18:51:37 +0300 Subject: [PATCH 6/6] [#49] util/proto: Do not allocate in StringSize() It was not catched by the test because most of the time the function is inlined. However, I've seen it allocating with pprof in one of the earlier builds. Signed-off-by: Evgenii Stratonikov --- util/proto/marshal.go | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/util/proto/marshal.go b/util/proto/marshal.go index e345492..b1f55a3 100644 --- a/util/proto/marshal.go +++ b/util/proto/marshal.go @@ -20,13 +20,21 @@ type ( ) func BytesMarshal(field int, buf, v []byte) int { - if len(v) == 0 { - return 0 - } return bytesMarshal(field, buf, v) } -func bytesMarshal(field int, buf, v []byte) int { +func BytesSize(field int, v []byte) int { + return bytesSize(field, v) +} + +func bytesMarshal[T ~[]byte | ~string](field int, buf []byte, v T) int { + if len(v) == 0 { + return 0 + } + return bytesMarshalNoCheck(field, buf, v) +} + +func bytesMarshalNoCheck[T ~[]byte | ~string](field int, buf []byte, v T) int { prefix := field<<3 | 0x2 // buf length check can prevent panic at PutUvarint, but it will make @@ -38,26 +46,25 @@ func bytesMarshal(field int, buf, v []byte) int { return i } -func BytesSize(field int, v []byte) int { - ln := len(v) - if ln == 0 { +func bytesSize[T ~[]byte | ~string](field int, v T) int { + if len(v) == 0 { return 0 } - return bytesSize(field, v) + return bytesSizeNoCheck(field, v) } -func bytesSize(field int, v []byte) int { +func bytesSizeNoCheck[T ~[]byte | ~string](field int, v T) int { prefix := field<<3 | 0x2 return VarUIntSize(uint64(prefix)) + VarUIntSize(uint64(len(v))) + len(v) } func StringMarshal(field int, buf []byte, v string) int { - return BytesMarshal(field, buf, []byte(v)) + return bytesMarshal(field, buf, v) } func StringSize(field int, v string) int { - return BytesSize(field, []byte(v)) + return bytesSize(field, v) } func BoolMarshal(field int, buf []byte, v bool) int { @@ -146,7 +153,7 @@ func RepeatedBytesMarshal(field int, buf []byte, v [][]byte) int { var offset int for i := range v { - offset += bytesMarshal(field, buf[offset:], v[i]) + offset += bytesMarshalNoCheck(field, buf[offset:], v[i]) } return offset @@ -154,7 +161,7 @@ func RepeatedBytesMarshal(field int, buf []byte, v [][]byte) int { func RepeatedBytesSize(field int, v [][]byte) (size int) { for i := range v { - size += bytesSize(field, v[i]) + size += bytesSizeNoCheck(field, v[i]) } return size @@ -164,7 +171,7 @@ func RepeatedStringMarshal(field int, buf []byte, v []string) int { var offset int for i := range v { - offset += bytesMarshal(field, buf[offset:], []byte(v[i])) + offset += bytesMarshalNoCheck(field, buf[offset:], v[i]) } return offset @@ -172,7 +179,7 @@ func RepeatedStringMarshal(field int, buf []byte, v []string) int { func RepeatedStringSize(field int, v []string) (size int) { for i := range v { - size += bytesSize(field, []byte(v[i])) + size += bytesSizeNoCheck(field, v[i]) } return size -- 2.45.3