From b46a319870d3302230a5e388fc2dfd632f856761 Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Fri, 26 Sep 2025 18:18:44 +0200 Subject: [PATCH 1/2] accounts/keystore: introduce accountCache.add and accountCache.find benchmarks For #16874 --- accounts/keystore/account_cache_test.go | 91 +++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/accounts/keystore/account_cache_test.go b/accounts/keystore/account_cache_test.go index c9a8cdfcef..5ba7211d18 100644 --- a/accounts/keystore/account_cache_test.go +++ b/accounts/keystore/account_cache_test.go @@ -17,6 +17,7 @@ package keystore import ( + "encoding/binary" "errors" "fmt" "math/rand" @@ -405,3 +406,93 @@ func forceCopyFile(dst, src string) error { } return os.WriteFile(dst, data, 0644) } + +func BenchmarkAdd(b *testing.B) { + for _, preload := range []int{10, 100, 1000, 1_000_000} { + b.Run(fmt.Sprintf("preload=%d", preload), func(b *testing.B) { + benchmarkAdd(b, preload) + }) + } +} + +func benchmarkAdd(b *testing.B, preload int) { + dir := filepath.Join("testdata", "dir") + cache, _ := newAccountCache(dir) + cache.watcher.running = true // prevent unexpected reloads + + for i := range preload { + acc := accounts.Account{ + URL: accounts.URL{Scheme: KeyStoreScheme, Path: fmt.Sprintf("dir/preload%08x", i)}, + } + binary.NativeEndian.PutUint64(acc.Address[0:], uint64(i)) + + cache.add(acc) + } + + b.ResetTimer() + b.ReportAllocs() + for i := range b.N { + acc := accounts.Account{ + URL: accounts.URL{Scheme: KeyStoreScheme, Path: fmt.Sprintf("dir/bench%08x", i)}, + } + binary.NativeEndian.PutUint64(acc.Address[12:], uint64(i)) + + cache.add(acc) + } +} + +func BenchmarkFind(b *testing.B) { + for _, preload := range []int{10, 100, 1000, 1_000_000} { + b.Run(fmt.Sprintf("preload=%d", preload), func(b *testing.B) { + benchmarkFind(b, preload) + }) + } +} + +func benchmarkFind(b *testing.B, preload int) { + dir := filepath.Join("testdata", "dir") + cache, _ := newAccountCache(dir) + cache.watcher.running = true // prevent unexpected reloads + + for i := range preload { + acc := accounts.Account{ + URL: accounts.URL{Scheme: KeyStoreScheme, Path: fmt.Sprintf("dir/account%08x", i)}, + } + binary.NativeEndian.PutUint64(acc.Address[0:], uint64(i)) + + cache.add(acc) + } + + b.Run("by address", func(b *testing.B) { + acc := accounts.Account{} + binary.NativeEndian.PutUint64(acc.Address[0:], uint64(preload/2)) + + b.ResetTimer() + b.ReportAllocs() + for range b.N { + cache.find(acc) + } + }) + + b.Run("by path", func(b *testing.B) { + acc := accounts.Account{ + URL: accounts.URL{Scheme: KeyStoreScheme, Path: fmt.Sprintf("dir/account%08x", preload/2)}, + } + + b.ResetTimer() + b.ReportAllocs() + for range b.N { + cache.find(acc) + } + }) + + b.Run("ambiguous", func(b *testing.B) { + acc := accounts.Account{} + + b.ResetTimer() + b.ReportAllocs() + for range b.N { + cache.find(acc) + } + }) +} From a00430d4aa75f62861cc58bfdd1994046cbbec89 Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Fri, 26 Sep 2025 18:18:44 +0200 Subject: [PATCH 2/2] accounts/keystore: use map instead of slice to keep all accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After profiling geth startup times for #16874 it turned out that most of the time was spent resizing accounts slice for ordered insertion. This change uses map to track all accounts instead of slice to improve performance. geth startup time using keystore with one million accounts reduced from 23 minutes to 2.5 minutes. Benchmarks show relatively-small overhead increase for small keystores and large decrease for huge: ``` goos: linux goarch: amd64 pkg: github.com/ethereum/go-ethereum/accounts/keystore │ HEAD~1 │ HEAD │ │ sec/op │ sec/op vs base │ Add/preload=10-8 1.030µ ± 5% 1.447µ ± 7% +40.51% (p=0.000 n=10) Add/preload=100-8 1.109µ ± 4% 1.463µ ± 3% +31.88% (p=0.000 n=10) Add/preload=1000-8 1.860µ ± 3% 1.477µ ± 5% -20.57% (p=0.000 n=10) Add/preload=1000000-8 5177.640µ ± 2% 1.654µ ± 11% -99.97% (p=0.000 n=10) Find/preload=10/by_address-8 23.70n ± 1% 23.88n ± 3% ~ (p=0.271 n=10) Find/preload=10/by_path-8 50.43n ± 2% 39.88n ± 5% -20.94% (p=0.000 n=10) Find/preload=10/ambiguous-8 323.6n ± 1% 1049.0n ± 3% +224.17% (p=0.000 n=10) Find/preload=100/by_address-8 23.69n ± 1% 23.63n ± 6% ~ (p=0.739 n=10) Find/preload=100/by_path-8 362.70n ± 1% 37.84n ± 3% -89.57% (p=0.000 n=10) Find/preload=100/ambiguous-8 2.683µ ± 2% 19.235µ ± 2% +617.05% (p=0.000 n=10) Find/preload=1000/by_address-8 26.45n ± 1% 27.73n ± 2% +4.82% (p=0.000 n=10) Find/preload=1000/by_path-8 3211.00n ± 3% 38.22n ± 8% -98.81% (p=0.000 n=10) Find/preload=1000/ambiguous-8 26.14µ ± 2% 263.59µ ± 1% +908.41% (p=0.000 n=10) Find/preload=1000000/by_address-8 26.47n ± 4% 26.41n ± 1% ~ (p=0.566 n=10) Find/preload=1000000/by_path-8 3683325.50n ± 4% 44.09n ± 45% -100.00% (p=0.000 n=10) Find/preload=1000000/ambiguous-8 39.68m ± 14% 819.48m ± 7% +1965.01% (p=0.000 n=10) geomean 2.346µ 791.4n -66.27% │ HEAD~1 │ HEAD │ │ B/op │ B/op vs base │ Add/preload=10-8 643.0 ± 0% 662.0 ± 0% +2.95% (p=0.000 n=10) Add/preload=100-8 643.0 ± 0% 662.0 ± 0% +2.95% (p=0.000 n=10) Add/preload=1000-8 584.0 ± 5% 662.0 ± 0% +13.36% (p=0.000 n=10) Add/preload=1000000-8 88.00 ± 0% 662.00 ± 17% +652.27% (p=0.000 n=10) Find/preload=10/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=10/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=10/ambiguous-8 624.0 ± 0% 1200.0 ± 0% +92.31% (p=0.000 n=10) Find/preload=100/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=100/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=100/ambiguous-8 6.047Ki ± 0% 12.047Ki ± 0% +99.22% (p=0.000 n=10) Find/preload=1000/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000/ambiguous-8 56.05Ki ± 0% 112.05Ki ± 0% +99.92% (p=0.000 n=10) Find/preload=1000000/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000000/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000000/ambiguous-8 53.41Mi ± 0% 106.81Mi ± 0% +100.00% (p=0.000 n=10) geomean ² +36.09% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ HEAD~1 │ HEAD │ │ allocs/op │ allocs/op vs base │ Add/preload=10-8 3.000 ± 0% 4.000 ± 0% +33.33% (p=0.000 n=10) Add/preload=100-8 3.000 ± 0% 4.000 ± 0% +33.33% (p=0.000 n=10) Add/preload=1000-8 3.000 ± 0% 4.000 ± 0% +33.33% (p=0.000 n=10) Add/preload=1000000-8 2.000 ± 0% 4.000 ± 0% +100.00% (p=0.000 n=10) Find/preload=10/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=10/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=10/ambiguous-8 2.000 ± 0% 3.000 ± 0% +50.00% (p=0.000 n=10) Find/preload=100/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=100/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=100/ambiguous-8 2.000 ± 0% 3.000 ± 0% +50.00% (p=0.000 n=10) Find/preload=1000/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000/ambiguous-8 2.000 ± 0% 3.000 ± 0% +50.00% (p=0.000 n=10) Find/preload=1000000/by_address-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000000/by_path-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Find/preload=1000000/ambiguous-8 2.000 ± 0% 3.000 ± 0% +50.00% (p=0.000 n=10) geomean ² +21.97% ² ¹ all samples are equal ² summaries must be >0 to compute geomean ``` Updates #16874 --- accounts/keystore/account_cache.go | 67 +++++++++++++++++++----------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/accounts/keystore/account_cache.go b/accounts/keystore/account_cache.go index d3a98850c7..f2db185c3d 100644 --- a/accounts/keystore/account_cache.go +++ b/accounts/keystore/account_cache.go @@ -66,7 +66,7 @@ type accountCache struct { keydir string watcher *watcher mu sync.Mutex - all []accounts.Account + byURL map[accounts.URL][]accounts.Account byAddr map[common.Address][]accounts.Account throttle *time.Timer notify chan struct{} @@ -76,6 +76,7 @@ type accountCache struct { func newAccountCache(keydir string) (*accountCache, chan struct{}) { ac := &accountCache{ keydir: keydir, + byURL: make(map[accounts.URL][]accounts.Account), byAddr: make(map[common.Address][]accounts.Account), notify: make(chan struct{}, 1), fileC: fileCache{all: mapset.NewThreadUnsafeSet[string]()}, @@ -88,8 +89,11 @@ func (ac *accountCache) accounts() []accounts.Account { ac.maybeReload() ac.mu.Lock() defer ac.mu.Unlock() - cpy := make([]accounts.Account, len(ac.all)) - copy(cpy, ac.all) + cpy := make([]accounts.Account, 0, len(ac.byURL)) + for _, accs := range ac.byURL { + cpy = append(cpy, accs...) + } + sort.SliceStable(cpy, func(i, j int) bool { return cpy[i].URL.Cmp(cpy[j].URL) < 0 }) return cpy } @@ -104,14 +108,11 @@ func (ac *accountCache) add(newAccount accounts.Account) { ac.mu.Lock() defer ac.mu.Unlock() - i := sort.Search(len(ac.all), func(i int) bool { return ac.all[i].URL.Cmp(newAccount.URL) >= 0 }) - if i < len(ac.all) && ac.all[i] == newAccount { + if accs, ok := ac.byURL[newAccount.URL]; ok && slices.Contains(accs, newAccount) { return } // newAccount is not in the cache. - ac.all = append(ac.all, accounts.Account{}) - copy(ac.all[i+1:], ac.all[i:]) - ac.all[i] = newAccount + ac.byURL[newAccount.URL] = append(ac.byURL[newAccount.URL], newAccount) ac.byAddr[newAccount.Address] = append(ac.byAddr[newAccount.Address], newAccount) } @@ -120,7 +121,12 @@ func (ac *accountCache) delete(removed accounts.Account) { ac.mu.Lock() defer ac.mu.Unlock() - ac.all = removeAccount(ac.all, removed) + if bu := removeAccount(ac.byURL[removed.URL], removed); len(bu) == 0 { + delete(ac.byURL, removed.URL) + } else { + ac.byURL[removed.URL] = bu + } + if ba := removeAccount(ac.byAddr[removed.Address], removed); len(ba) == 0 { delete(ac.byAddr, removed.Address) } else { @@ -132,11 +138,14 @@ func (ac *accountCache) delete(removed accounts.Account) { func (ac *accountCache) deleteByFile(path string) { ac.mu.Lock() defer ac.mu.Unlock() - i := sort.Search(len(ac.all), func(i int) bool { return ac.all[i].URL.Path >= path }) - - if i < len(ac.all) && ac.all[i].URL.Path == path { - removed := ac.all[i] - ac.all = append(ac.all[:i], ac.all[i+1:]...) + url := accounts.URL{Scheme: KeyStoreScheme, Path: path} + if accs, ok := ac.byURL[url]; ok { + removed := accs[0] + if len(accs) == 1 { + delete(ac.byURL, url) + } else { + ac.byURL[url] = accs[1:] + } if ba := removeAccount(ac.byAddr[removed.Address], removed); len(ba) == 0 { delete(ac.byAddr, removed.Address) } else { @@ -166,24 +175,34 @@ func removeAccount(slice []accounts.Account, elem accounts.Account) []accounts.A // The exact matching rules are explained by the documentation of accounts.Account. // Callers must hold ac.mu. func (ac *accountCache) find(a accounts.Account) (accounts.Account, error) { - // Limit search to address candidates if possible. - matches := ac.all - if (a.Address != common.Address{}) { - matches = ac.byAddr[a.Address] - } if a.URL.Path != "" { // If only the basename is specified, complete the path. if !strings.ContainsRune(a.URL.Path, filepath.Separator) { a.URL.Path = filepath.Join(ac.keydir, a.URL.Path) } - for i := range matches { - if matches[i].URL == a.URL { - return matches[i], nil + } + // Limit search to address candidates if possible. + var matches []accounts.Account + if (a.Address != common.Address{}) { + matches = ac.byAddr[a.Address] + if a.URL.Path != "" { + for i := range matches { + if matches[i].URL == a.URL { + return matches[i], nil + } } } - if (a.Address == common.Address{}) { + } else { + if a.URL.Path != "" { + if accs, ok := ac.byURL[a.URL]; ok { + return accs[0], nil + } return accounts.Account{}, ErrNoMatch } + matches = make([]accounts.Account, 0, len(ac.byURL)) + for _, accs := range ac.byURL { + matches = append(matches, accs...) + } } switch len(matches) { case 1: @@ -193,7 +212,7 @@ func (ac *accountCache) find(a accounts.Account) (accounts.Account, error) { default: err := &AmbiguousAddrError{Addr: a.Address, Matches: make([]accounts.Account, len(matches))} copy(err.Matches, matches) - slices.SortFunc(err.Matches, byURL) + slices.SortStableFunc(err.Matches, byURL) return accounts.Account{}, err } }