Skip to content

Commit

Permalink
starlark: remove allocs on Dict ranges for dictsEqual and write (goog…
Browse files Browse the repository at this point in the history
…le#402)

Drops Items() range in favour of looking through the hashtable list
directly. This avoids an allocation, added a benchmark to verify.

Old:
Benchmark/bench_dict_equal-4               18782             61652 ns/op           57344 B/op          2 allocs/op

New:
Benchmark/bench_dict_equal-4               29802             39585 ns/op               0 B/op          0 allocs/op
  • Loading branch information
emcfarlane authored Mar 2, 2022
1 parent c8e9b32 commit 5411bad
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
8 changes: 8 additions & 0 deletions starlark/testdata/benchmark.star
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,11 @@ def bench_mix(b):
if i:
x += 1
a = [x for x in range(i)]

largedict = {str(v): v for v in range(1000)}

def bench_dict_equal(b):
"Benchmark of dict equality operation."
for _ in range(b.n):
if largedict != largedict:
fail("invalid comparison")
8 changes: 4 additions & 4 deletions starlark/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,8 +816,8 @@ func dictsEqual(x, y *Dict, depth int) (bool, error) {
if x.Len() != y.Len() {
return false, nil
}
for _, xitem := range x.Items() {
key, xval := xitem[0], xitem[1]
for e := x.ht.head; e != nil; e = e.next {
key, xval := e.key, e.value

if yval, found, _ := y.Get(key); !found {
return false, nil
Expand Down Expand Up @@ -1185,8 +1185,8 @@ func writeValue(out *strings.Builder, x Value, path []Value) {
out.WriteString("...") // dict contains itself
} else {
sep := ""
for _, item := range x.Items() {
k, v := item[0], item[1]
for e := x.ht.head; e != nil; e = e.next {
k, v := e.key, e.value
out.WriteString(sep)
writeValue(out, k, path)
out.WriteString(": ")
Expand Down

0 comments on commit 5411bad

Please sign in to comment.