From 6f4908edc23cce4d3d63b9c17bace4b70dc8f427 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 2 Mar 2022 10:40:22 +0300 Subject: [PATCH] [#376] netmap: Make attributes a non-pointer slice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The speed of copying (which is done regulary for e.g. subnet changes) is less, however it isn't on the hot path and the absolute time difference is insignificant. ``` name old time/op new time/op delta NodeAttributes-8 96.2ns ± 1% 158.3ns ± 1% +64.61% (p=0.000 n=10+10) name old alloc/op new alloc/op delta NodeAttributes-8 32.0B ± 0% 32.0B ± 0% ~ (all equal) name old allocs/op new allocs/op delta NodeAttributes-8 2.00 ± 0% 2.00 ± 0% ~ (all equal) ``` Signed-off-by: Evgenii Stratonikov --- netmap/attributes.go | 20 +++++------ netmap/attributes_test.go | 70 ++++++++++++++++++++++++++++++++++----- netmap/convert.go | 14 +++----- netmap/marshal.go | 4 +-- netmap/test/generate.go | 8 ++--- netmap/types.go | 6 ++-- 6 files changed, 82 insertions(+), 40 deletions(-) diff --git a/netmap/attributes.go b/netmap/attributes.go index 7bfc002..05f4d4a 100644 --- a/netmap/attributes.go +++ b/netmap/attributes.go @@ -101,12 +101,10 @@ func WriteSubnetInfo(node *NodeInfo, info NodeSubnetInfo) { } if !presented { - var attr Attribute - - attr.SetKey(key) - attr.SetValue(val) - - attrs = append(attrs, &attr) + index := len(attrs) + attrs = append(attrs, Attribute{}) + attrs[index].SetKey(key) + attrs[index].SetValue(val) } } @@ -208,12 +206,10 @@ func IterateSubnets(node *NodeInfo, f func(refs.SubnetID) error) error { } // zero subnet should be clearly removed with False value - var attr Attribute - - attr.SetKey(subnetAttributeKey(&id)) - attr.SetValue(attrSubnetValExit) - - attrs = append(attrs, &attr) + index := len(attrs) + attrs = append(attrs, Attribute{}) + attrs[index].SetKey(subnetAttributeKey(&id)) + attrs[index].SetValue(attrSubnetValExit) } else { entries++ } diff --git a/netmap/attributes_test.go b/netmap/attributes_test.go index 93586ac..e485dac 100644 --- a/netmap/attributes_test.go +++ b/netmap/attributes_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/nspcc-dev/neofs-api-go/v2/netmap" + netmaptest "github.com/nspcc-dev/neofs-api-go/v2/netmap/test" "github.com/nspcc-dev/neofs-api-go/v2/refs" "github.com/stretchr/testify/require" ) @@ -17,6 +18,57 @@ func assertSubnetAttrKey(t *testing.T, attr *netmap.Attribute, num uint32) { require.Equal(t, subnetAttrKey(strconv.FormatUint(uint64(num), 10)), attr.GetKey()) } +func BenchmarkNodeAttributes(b *testing.B) { + const size = 50 + + id := new(refs.SubnetID) + id.SetValue(12) + + attrs := make([]netmap.Attribute, size) + for i := range attrs { + if i == size/2 { + attrs[i] = *netmaptest.GenerateAttribute(false) + } else { + data, err := id.MarshalText() + require.NoError(b, err) + + attrs[i].SetKey(subnetAttrKey(string(data))) + attrs[i].SetValue("True") + } + } + + var info netmap.NodeSubnetInfo + info.SetID(id) + info.SetEntryFlag(false) + + node := new(netmap.NodeInfo) + + // When using a single slice `StartTimer` overhead is comparable to the + // function execution time, so we reduce this cost by updating slices in groups. + const cacheSize = 1000 + a := make([][]netmap.Attribute, cacheSize) + for i := range a { + a[i] = make([]netmap.Attribute, size) + } + + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + if i%cacheSize == 0 { + b.StopTimer() + for j := range a { + copy(a[j], attrs) + } + b.StartTimer() + } + node.SetAttributes(a[i%cacheSize]) + netmap.WriteSubnetInfo(node, info) + if len(node.GetAttributes())+1 != len(attrs) { + b.FailNow() + } + } +} + func TestWriteSubnetInfo(t *testing.T) { t.Run("entry", func(t *testing.T) { t.Run("zero subnet", func(t *testing.T) { @@ -40,7 +92,7 @@ func TestWriteSubnetInfo(t *testing.T) { attrs = node.GetAttributes() require.Len(t, attrs, 1) - attr := attrs[0] + attr := &attrs[0] assertSubnetAttrKey(t, attr, 0) require.Equal(t, "False", attr.GetValue()) @@ -76,7 +128,7 @@ func TestWriteSubnetInfo(t *testing.T) { attrs := node.GetAttributes() require.Len(t, attrs, 1) - attr := attrs[0] + attr := &attrs[0] assertSubnetAttrKey(t, attr, num) require.Equal(t, "True", attr.GetValue()) @@ -128,7 +180,7 @@ func TestSubnets(t *testing.T) { attrExit.SetKey(subnetAttrKey(strconv.FormatUint(numExit, 10))) attrExit.SetValue("False") - attrs := []*netmap.Attribute{&attrEntry, &attrEntry} + attrs := []netmap.Attribute{attrEntry, attrEntry} node.SetAttributes(attrs) @@ -157,7 +209,7 @@ func TestSubnets(t *testing.T) { assertErr := func(attr netmap.Attribute) { var node netmap.NodeInfo - node.SetAttributes([]*netmap.Attribute{&attr}) + node.SetAttributes([]netmap.Attribute{attr}) require.Error(t, netmap.IterateSubnets(&node, func(refs.SubnetID) error { return nil @@ -200,7 +252,7 @@ func TestSubnets(t *testing.T) { attr.SetKey(subnetAttrKey("321")) attr.SetValue("True") - attrs := []*netmap.Attribute{&attr} + attrs := []netmap.Attribute{attr} node.SetAttributes(attrs) err := netmap.IterateSubnets(&node, func(id refs.SubnetID) error { @@ -237,7 +289,7 @@ func TestSubnets(t *testing.T) { attr.SetKey(subnetAttrKey("99")) attr.SetValue("True") - attrs := []*netmap.Attribute{&attr} + attrs := []netmap.Attribute{attr} node.SetAttributes(attrs) err := netmap.IterateSubnets(&node, func(id refs.SubnetID) error { @@ -257,7 +309,7 @@ func TestSubnets(t *testing.T) { t.Run("all", func(t *testing.T) { var ( node netmap.NodeInfo - attrs []*netmap.Attribute + attrs []netmap.Attribute ) // enter to some non-zero subnet so that zero is not the only one @@ -267,7 +319,7 @@ func TestSubnets(t *testing.T) { attr.SetKey(subnetAttrKey(strconv.Itoa(i))) attr.SetValue("True") - attrs = append(attrs, &attr) + attrs = append(attrs, attr) } node.SetAttributes(attrs) @@ -293,7 +345,7 @@ func TestSubnets(t *testing.T) { attrOther.SetKey(subnetAttrKey("1")) attrOther.SetValue("True") - node.SetAttributes([]*netmap.Attribute{&attrZero, &attrOther}) + node.SetAttributes([]netmap.Attribute{attrZero, attrOther}) calledCount := 0 diff --git a/netmap/convert.go b/netmap/convert.go index 2f1f68b..14c3792 100644 --- a/netmap/convert.go +++ b/netmap/convert.go @@ -297,7 +297,7 @@ func (a *Attribute) FromGRPCMessage(m grpc.Message) error { return nil } -func AttributesToGRPC(as []*Attribute) (res []*netmap.NodeInfo_Attribute) { +func AttributesToGRPC(as []Attribute) (res []*netmap.NodeInfo_Attribute) { if as != nil { res = make([]*netmap.NodeInfo_Attribute, 0, len(as)) @@ -309,23 +309,17 @@ func AttributesToGRPC(as []*Attribute) (res []*netmap.NodeInfo_Attribute) { return } -func AttributesFromGRPC(as []*netmap.NodeInfo_Attribute) (res []*Attribute, err error) { +func AttributesFromGRPC(as []*netmap.NodeInfo_Attribute) (res []Attribute, err error) { if as != nil { - res = make([]*Attribute, 0, len(as)) + res = make([]Attribute, len(as)) for i := range as { - var a *Attribute - if as[i] != nil { - a = new(Attribute) - - err = a.FromGRPCMessage(as[i]) + err = res[i].FromGRPCMessage(as[i]) if err != nil { return } } - - res = append(res, a) } } diff --git a/netmap/marshal.go b/netmap/marshal.go index 165af23..70a8b4c 100644 --- a/netmap/marshal.go +++ b/netmap/marshal.go @@ -383,7 +383,7 @@ func (ni *NodeInfo) StableMarshal(buf []byte) ([]byte, error) { offset += n for i := range ni.attributes { - n, err = protoutil.NestedStructureMarshal(attributesNodeInfoField, buf[offset:], ni.attributes[i]) + n, err = protoutil.NestedStructureMarshal(attributesNodeInfoField, buf[offset:], &ni.attributes[i]) if err != nil { return nil, err } @@ -408,7 +408,7 @@ func (ni *NodeInfo) StableSize() (size int) { size += protoutil.RepeatedStringSize(addressNodeInfoField, ni.addresses) for i := range ni.attributes { - size += protoutil.NestedStructureSize(attributesNodeInfoField, ni.attributes[i]) + size += protoutil.NestedStructureSize(attributesNodeInfoField, &ni.attributes[i]) } size += protoutil.EnumSize(stateNodeInfoField, int32(ni.state)) diff --git a/netmap/test/generate.go b/netmap/test/generate.go index 0042dbb..7040550 100644 --- a/netmap/test/generate.go +++ b/netmap/test/generate.go @@ -119,13 +119,13 @@ func GenerateAttribute(empty bool) *netmap.Attribute { return m } -func GenerateAttributes(empty bool) []*netmap.Attribute { - var res []*netmap.Attribute +func GenerateAttributes(empty bool) []netmap.Attribute { + var res []netmap.Attribute if !empty { res = append(res, - GenerateAttribute(false), - GenerateAttribute(false), + *GenerateAttribute(false), + *GenerateAttribute(false), ) } diff --git a/netmap/types.go b/netmap/types.go index 5282765..19ca974 100644 --- a/netmap/types.go +++ b/netmap/types.go @@ -73,7 +73,7 @@ type Attribute struct { type NodeInfo struct { publicKey []byte addresses []string - attributes []*Attribute + attributes []Attribute state NodeState } @@ -444,7 +444,7 @@ func (ni *NodeInfo) IterateAddresses(f func(string) bool) { } } -func (ni *NodeInfo) GetAttributes() []*Attribute { +func (ni *NodeInfo) GetAttributes() []Attribute { if ni != nil { return ni.attributes } @@ -452,7 +452,7 @@ func (ni *NodeInfo) GetAttributes() []*Attribute { return nil } -func (ni *NodeInfo) SetAttributes(v []*Attribute) { +func (ni *NodeInfo) SetAttributes(v []Attribute) { if ni != nil { ni.attributes = v }