Skip to content

Commit

Permalink
types: intern AccAddress.String() to gut wasted expensive recomputati…
Browse files Browse the repository at this point in the history
…ons (cosmos#8694)

Given that AccAddress is a []byte type, and its .String() method is
quite expensive, continuously invoking the method doesn't easily provide
a way for the result to be memoized. In memory profiles from
benchmarking OneBankSendTxPerBlock and run for a while, it showed >2GiB burnt
and SendCoins consuming a bunch of memory: >2GiB.

This change introduces a simple global cache with a map to intern
AccAddress values, so that we quickly look up the expensively computed
values. With it, the prior memory profiles are erased, and benchmarks
show improvements:

```shell
$ benchstat before.txt after.txt
name                     old time/op    new time/op    delta
OneBankSendTxPerBlock-8    1.94ms ± 9%    1.92ms ±11%   -1.34%  (p=0.011 n=58+57)

name                     old alloc/op   new alloc/op   delta
OneBankSendTxPerBlock-8     428kB ± 0%     365kB ± 0%  -14.67%  (p=0.000 n=58+54)

name                     old allocs/op  new allocs/op  delta
OneBankSendTxPerBlock-8     6.34k ± 0%     5.90k ± 0%   -7.06%  (p=0.000 n=58+57)
```

Fixes cosmos#8693

Co-authored-by: Alessio Treglia <[email protected]>
  • Loading branch information
odeke-em and Alessio Treglia authored Feb 25, 2021
1 parent ba74a7c commit 929b62c
Showing 1 changed file with 22 additions and 1 deletion.
23 changes: 22 additions & 1 deletion types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"strings"
"sync"

yaml "gopkg.in/yaml.v2"

Expand Down Expand Up @@ -236,8 +237,28 @@ func (aa AccAddress) Bytes() []byte {
return aa
}

// AccAddress.String() is expensive and if unoptimized dominantly showed up in profiles,
// yet has no mechanisms to trivially cache the result given that AccAddress is a []byte type.
// With the string interning below, we are able to achieve zero cost allocations for string->[]byte
// because the Go compiler recognizes the special case of map[string([]byte)] when used exactly
// in that pattern. See https://github.com/cosmos/cosmos-sdk/issues/8693.
var addMu sync.Mutex
var addrStrMemoize = make(map[string]string)

// String implements the Stringer interface.
func (aa AccAddress) String() string {
func (aa AccAddress) String() (str string) {
addMu.Lock()
defer addMu.Unlock()

if str, ok := addrStrMemoize[string(aa)]; ok {
return str
}

// Otherwise cache it for later memoization.
defer func() {
addrStrMemoize[string(aa)] = str
}()

if aa.Empty() {
return ""
}
Expand Down

0 comments on commit 929b62c

Please sign in to comment.