From 8a7e96ae3d9d4fb9c72f81dbdf2851fa85dc2086 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Thu, 2 Jan 2025 10:06:11 -0800 Subject: [PATCH] perf(gnolang): use slice not map for Attributes.data per usage performance Noticed in profiling stdlibs/bytes that a ton of memory was being used in maps, and that's due to the conventional CS 101 that maps with O(1) lookups, insertions and deletions beat O(n) slices' performance, but when n is small, the memory bloat is not worth it and we can use slices as evidenced in profiles for which there was 30% perceptible reduction in RAM where * Before: ```shell Showing nodes accounting for 92.90MB, 83.87% of 110.76MB total Dropped 51 nodes (cum <= 0.55MB) Showing top 10 nodes out of 123 flat flat% sum% cum cum% 47.37MB 42.77% 42.77% 47.37MB 42.77% internal/runtime/maps.newarray 10.50MB 9.48% 52.25% 10.50MB 9.48% internal/runtime/maps.NewEmptyMap 8MB 7.22% 59.47% 8MB 7.22% github.com/gnolang/gno/gnovm/pkg/gnolang.(*StaticBlock).InitStaticBlock 7.51MB 6.78% 66.25% 13.03MB 11.76% github.com/gnolang/gno/gnovm/pkg/gnolang.Go2Gno 6.02MB 5.43% 71.68% 10.73MB 9.68% github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).SetObject 4MB 3.61% 75.29% 4MB 3.61% github.com/gnolang/gno/gnovm/pkg/gnolang.NewBlock 3.47MB 3.13% 78.43% 3.47MB 3.13% github.com/gnolang/gno/gnovm/pkg/gnolang.(*Allocator).NewDataArray 2.52MB 2.27% 80.70% 3.52MB 3.18% github.com/gnolang/gno/gnovm/pkg/gnolang.toKeyValueExprs 2MB 1.81% 82.51% 2MB 1.81% runtime.allocm 1.51MB 1.36% 83.87% 1.51MB 1.36% runtime/pprof.(*profMap).lookup ``` ```shell Showing nodes accounting for 47.37MB, 42.77% of 110.76MB total ----------------------------------------------------------+------------- flat flat% sum% cum cum% calls calls% + context ----------------------------------------------------------+------------- 47.37MB 100% | internal/runtime/maps.newGroups 47.37MB 42.77% 42.77% 47.37MB 42.77% | internal/runtime/maps.newarray ----------------------------------------------------------+------------- 32.01MB 78.05% | github.com/gnolang/gno/gnovm/pkg/gnolang.preprocess1.func1 7MB 17.07% | github.com/gnolang/gno/gnovm/pkg/gnolang.evalConst (inline) 1.50MB 3.66% | github.com/gnolang/gno/gnovm/pkg/gnolang.constType (inline) 0.50MB 1.22% | github.com/gnolang/gno/gnovm/pkg/gnolang.tryPredefine.func1 0 0% 42.77% 41.01MB 37.03% | github.com/gnolang/gno/gnovm/pkg/gnolang.(*Attributes).SetAttribute 41.01MB 100% | runtime.mapassign_faststr ----------------------------------------------------------+------------- 4.50MB 100% | github.com/gnolang/gno/gnovm/pkg/test.(*TestOptions).runTestFiles 0 0% 42.77% 4.50MB 4.06% | github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).RunFiles 4.50MB 100% | github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).runFileDecls ``` and after: ```shell Showing nodes accounting for 61.99MB, 73.12% of 84.78MB total Showing top 10 nodes out of 196 flat flat% sum% cum cum% 19.50MB 23.00% 23.00% 19.50MB 23.00% github.com/gnolang/gno/gnovm/pkg/gnolang.(*Attributes).SetAttribute 12.52MB 14.76% 37.77% 18.02MB 21.26% github.com/gnolang/gno/gnovm/pkg/gnolang.Go2Gno 7.58MB 8.94% 46.70% 9.15MB 10.79% github.com/gnolang/gno/gnovm/pkg/gnolang.(*defaultStore).SetObject 5MB 5.90% 52.60% 5MB 5.90% github.com/gnolang/gno/gnovm/pkg/gnolang.(*StaticBlock).InitStaticBlock 3.47MB 4.09% 56.69% 3.47MB 4.09% github.com/gnolang/gno/gnovm/pkg/gnolang.(*Allocator).NewDataArray 3MB 3.54% 60.24% 3MB 3.54% github.com/gnolang/gno/gnovm/pkg/gnolang.NewBlock 3MB 3.54% 63.77% 3MB 3.54% github.com/gnolang/gno/gnovm/pkg/gnolang.Nx (inline) 2.77MB 3.26% 67.04% 2.77MB 3.26% bytes.growSlice 2.65MB 3.12% 70.16% 2.65MB 3.12% internal/runtime/maps.newarray 2.50MB 2.95% 73.12% 2.50MB 2.95% runtime.allocm ``` Fixes #3436 --- gnovm/pkg/gnolang/nodes.go | 46 ++++++++++++++++++++++---- gnovm/pkg/gnolang/nodes_test.go | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/gnovm/pkg/gnolang/nodes.go b/gnovm/pkg/gnolang/nodes.go index 0496d37ed72..156ac133df6 100644 --- a/gnovm/pkg/gnolang/nodes.go +++ b/gnovm/pkg/gnolang/nodes.go @@ -165,7 +165,7 @@ type Attributes struct { Line int Column int Label Name - data map[GnoAttribute]interface{} // not persisted + data []*attrKV // not persisted } func (attr *Attributes) GetLine() int { @@ -193,28 +193,62 @@ func (attr *Attributes) SetLabel(label Name) { } func (attr *Attributes) HasAttribute(key GnoAttribute) bool { - _, ok := attr.data[key] + _, _, ok := attr.getAttribute(key) return ok } // GnoAttribute must not be user provided / arbitrary, // otherwise will create potential exploits. func (attr *Attributes) GetAttribute(key GnoAttribute) interface{} { - return attr.data[key] + val, _, _ := attr.getAttribute(key) + return val +} + +func (attr *Attributes) getAttribute(key GnoAttribute) (any, int, bool) { + for i, kv := range attr.data { + if kv.key == key { + return kv.value, i, true + } + } + return nil, -1, false +} + +type attrKV struct { + key GnoAttribute + value any } func (attr *Attributes) SetAttribute(key GnoAttribute, value interface{}) { if attr.data == nil { - attr.data = make(map[GnoAttribute]interface{}) + attr.data = make([]*attrKV, 0, 4) } - attr.data[key] = value + + for _, kv := range attr.data { + if kv.key == key { + kv.value = value + return + } + } + + attr.data = append(attr.data, &attrKV{key, value}) } func (attr *Attributes) DelAttribute(key GnoAttribute) { if debug && attr.data == nil { panic("should not happen, attribute is expected to be non-empty.") } - delete(attr.data, key) + _, index, _ := attr.getAttribute(key) + if index < 0 { + return + } + + if index == 0 { + attr.data = attr.data[1:] + } else if index == len(attr.data)-1 { + attr.data = attr.data[:len(attr.data)-1] + } else { + attr.data = append(attr.data[:index], attr.data[index+1:]...) + } } // ---------------------------------------- diff --git a/gnovm/pkg/gnolang/nodes_test.go b/gnovm/pkg/gnolang/nodes_test.go index 2c3a03d8c09..9f057482324 100644 --- a/gnovm/pkg/gnolang/nodes_test.go +++ b/gnovm/pkg/gnolang/nodes_test.go @@ -1,6 +1,7 @@ package gnolang_test import ( + "fmt" "math" "testing" @@ -42,3 +43,59 @@ func TestStaticBlock_Define2_MaxNames(t *testing.T) { // This one should panic because the maximum number of names has been reached. staticBlock.Define2(false, gnolang.Name("a"), gnolang.BoolType, gnolang.TypedValue{T: gnolang.BoolType}) } + +func TestAttributesSetGetDel(t *testing.T) { + attrs := new(gnolang.Attributes) + if got, want := attrs.GetAttribute("a"), (any)(nil); got != want { + t.Errorf(".Get returned an unexpected value=%v, want=%v", got, want) + } + attrs.SetAttribute("a", 10) + if got, want := attrs.GetAttribute("a"), 10; got != want { + t.Errorf(".Get returned an unexpected value=%v, want=%v", got, want) + } + attrs.SetAttribute("a", 20) + if got, want := attrs.GetAttribute("a"), 20; got != want { + t.Errorf(".Get returned an unexpected value=%v, want=%v", got, want) + } + attrs.DelAttribute("a") + if got, want := attrs.GetAttribute("a"), (any)(nil); got != want { + t.Errorf(".Get returned an unexpected value=%v, want=%v", got, want) + } +} + +var sink any = nil + +func BenchmarkAttributesSetGetDel(b *testing.B) { + n := 100 + keys := make([]gnolang.GnoAttribute, 0, n) + for i := 0; i < n; i++ { + keys = append(keys, gnolang.GnoAttribute(fmt.Sprintf("%d", i))) + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + attrs := new(gnolang.Attributes) + for j := 0; j < 100; j++ { + sink = attrs.GetAttribute("a") + } + for j := 0; j < 100; j++ { + attrs.SetAttribute("a", j) + sink = attrs.GetAttribute("a") + } + + for j, key := range keys { + attrs.SetAttribute(key, j) + } + + for _, key := range keys { + sink = attrs.GetAttribute(key) + attrs.GetAttribute(key) + } + } + + if sink == nil { + b.Fatal("Benchmark did not run!") + } +}