Skip to content

Commit

Permalink
perf(gnolang): use slice not map for Attributes.data per usage perfor…
Browse files Browse the repository at this point in the history
…mance

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
  • Loading branch information
odeke-em committed Jan 2, 2025
1 parent beb48e7 commit 8a7e96a
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 6 deletions.
46 changes: 40 additions & 6 deletions gnovm/pkg/gnolang/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Check warning on line 196 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L196

Added line #L196 was not covered by tests
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:]...)
}

Check warning on line 251 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L250-L251

Added lines #L250 - L251 were not covered by tests
}

// ----------------------------------------
Expand Down
57 changes: 57 additions & 0 deletions gnovm/pkg/gnolang/nodes_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gnolang_test

import (
"fmt"
"math"
"testing"

Expand Down Expand Up @@ -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!")
}
}

0 comments on commit 8a7e96a

Please sign in to comment.