From 929b62cd4689f7353cff8a5d4324e67bb8e634c8 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Thu, 25 Feb 2021 03:01:03 -0800 Subject: [PATCH] types: intern AccAddress.String() to gut wasted expensive recomputations (#8694) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #8693 Co-authored-by: Alessio Treglia --- types/address.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/types/address.go b/types/address.go index 9f3f821605..8e6a8103f3 100644 --- a/types/address.go +++ b/types/address.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "strings" + "sync" yaml "gopkg.in/yaml.v2" @@ -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 "" }