Skip to content

Commit a00430d

Browse files
AlexanderYastrebovAlexanderYastrebov
committed
accounts/keystore: use map instead of slice to keep all accounts
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
1 parent b46a319 commit a00430d

File tree

1 file changed

+43
-24
lines changed

1 file changed

+43
-24
lines changed

‎accounts/keystore/account_cache.go‎

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ type accountCache struct {
6666
keydir string
6767
watcher *watcher
6868
mu sync.Mutex
69-
all []accounts.Account
69+
byURL map[accounts.URL][]accounts.Account
7070
byAddr map[common.Address][]accounts.Account
7171
throttle *time.Timer
7272
notify chan struct{}
@@ -76,6 +76,7 @@ type accountCache struct {
7676
func newAccountCache(keydir string) (*accountCache, chan struct{}) {
7777
ac := &accountCache{
7878
keydir: keydir,
79+
byURL: make(map[accounts.URL][]accounts.Account),
7980
byAddr: make(map[common.Address][]accounts.Account),
8081
notify: make(chan struct{}, 1),
8182
fileC: fileCache{all: mapset.NewThreadUnsafeSet[string]()},
@@ -88,8 +89,11 @@ func (ac *accountCache) accounts() []accounts.Account {
8889
ac.maybeReload()
8990
ac.mu.Lock()
9091
defer ac.mu.Unlock()
91-
cpy := make([]accounts.Account, len(ac.all))
92-
copy(cpy, ac.all)
92+
cpy := make([]accounts.Account, 0, len(ac.byURL))
93+
for _, accs := range ac.byURL {
94+
cpy = append(cpy, accs...)
95+
}
96+
sort.SliceStable(cpy, func(i, j int) bool { return cpy[i].URL.Cmp(cpy[j].URL) < 0 })
9397
return cpy
9498
}
9599

@@ -104,14 +108,11 @@ func (ac *accountCache) add(newAccount accounts.Account) {
104108
ac.mu.Lock()
105109
defer ac.mu.Unlock()
106110

107-
i := sort.Search(len(ac.all), func(i int) bool { return ac.all[i].URL.Cmp(newAccount.URL) >= 0 })
108-
if i < len(ac.all) && ac.all[i] == newAccount {
111+
if accs, ok := ac.byURL[newAccount.URL]; ok && slices.Contains(accs, newAccount) {
109112
return
110113
}
111114
// newAccount is not in the cache.
112-
ac.all = append(ac.all, accounts.Account{})
113-
copy(ac.all[i+1:], ac.all[i:])
114-
ac.all[i] = newAccount
115+
ac.byURL[newAccount.URL] = append(ac.byURL[newAccount.URL], newAccount)
115116
ac.byAddr[newAccount.Address] = append(ac.byAddr[newAccount.Address], newAccount)
116117
}
117118

@@ -120,7 +121,12 @@ func (ac *accountCache) delete(removed accounts.Account) {
120121
ac.mu.Lock()
121122
defer ac.mu.Unlock()
122123

123-
ac.all = removeAccount(ac.all, removed)
124+
if bu := removeAccount(ac.byURL[removed.URL], removed); len(bu) == 0 {
125+
delete(ac.byURL, removed.URL)
126+
} else {
127+
ac.byURL[removed.URL] = bu
128+
}
129+
124130
if ba := removeAccount(ac.byAddr[removed.Address], removed); len(ba) == 0 {
125131
delete(ac.byAddr, removed.Address)
126132
} else {
@@ -132,11 +138,14 @@ func (ac *accountCache) delete(removed accounts.Account) {
132138
func (ac *accountCache) deleteByFile(path string) {
133139
ac.mu.Lock()
134140
defer ac.mu.Unlock()
135-
i := sort.Search(len(ac.all), func(i int) bool { return ac.all[i].URL.Path >= path })
136-
137-
if i < len(ac.all) && ac.all[i].URL.Path == path {
138-
removed := ac.all[i]
139-
ac.all = append(ac.all[:i], ac.all[i+1:]...)
141+
url := accounts.URL{Scheme: KeyStoreScheme, Path: path}
142+
if accs, ok := ac.byURL[url]; ok {
143+
removed := accs[0]
144+
if len(accs) == 1 {
145+
delete(ac.byURL, url)
146+
} else {
147+
ac.byURL[url] = accs[1:]
148+
}
140149
if ba := removeAccount(ac.byAddr[removed.Address], removed); len(ba) == 0 {
141150
delete(ac.byAddr, removed.Address)
142151
} else {
@@ -166,24 +175,34 @@ func removeAccount(slice []accounts.Account, elem accounts.Account) []accounts.A
166175
// The exact matching rules are explained by the documentation of accounts.Account.
167176
// Callers must hold ac.mu.
168177
func (ac *accountCache) find(a accounts.Account) (accounts.Account, error) {
169-
// Limit search to address candidates if possible.
170-
matches := ac.all
171-
if (a.Address != common.Address{}) {
172-
matches = ac.byAddr[a.Address]
173-
}
174178
if a.URL.Path != "" {
175179
// If only the basename is specified, complete the path.
176180
if !strings.ContainsRune(a.URL.Path, filepath.Separator) {
177181
a.URL.Path = filepath.Join(ac.keydir, a.URL.Path)
178182
}
179-
for i := range matches {
180-
if matches[i].URL == a.URL {
181-
return matches[i], nil
183+
}
184+
// Limit search to address candidates if possible.
185+
var matches []accounts.Account
186+
if (a.Address != common.Address{}) {
187+
matches = ac.byAddr[a.Address]
188+
if a.URL.Path != "" {
189+
for i := range matches {
190+
if matches[i].URL == a.URL {
191+
return matches[i], nil
192+
}
182193
}
183194
}
184-
if (a.Address == common.Address{}) {
195+
} else {
196+
if a.URL.Path != "" {
197+
if accs, ok := ac.byURL[a.URL]; ok {
198+
return accs[0], nil
199+
}
185200
return accounts.Account{}, ErrNoMatch
186201
}
202+
matches = make([]accounts.Account, 0, len(ac.byURL))
203+
for _, accs := range ac.byURL {
204+
matches = append(matches, accs...)
205+
}
187206
}
188207
switch len(matches) {
189208
case 1:
@@ -193,7 +212,7 @@ func (ac *accountCache) find(a accounts.Account) (accounts.Account, error) {
193212
default:
194213
err := &AmbiguousAddrError{Addr: a.Address, Matches: make([]accounts.Account, len(matches))}
195214
copy(err.Matches, matches)
196-
slices.SortFunc(err.Matches, byURL)
215+
slices.SortStableFunc(err.Matches, byURL)
197216
return accounts.Account{}, err
198217
}
199218
}

0 commit comments

Comments
 (0)