Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/rawdb: allocate database keys with explicit size to avoid slice growth #27772

Merged
merged 2 commits into from
Aug 23, 2023

Conversation

MariusVanDerWijden
Copy link
Member

During profiling I noticed that a lot of allocations are spent trying to compute the keys for accessing the database:

ROUTINE ======================== github.com/ethereum/go-ethereum/core/rawdb.storageSnapshotKey in github.com/ethereum/go-ethereum/core/rawdb/schema.go
 288552922  288552922 (flat, cum)  2.96% of Total
         .          .    197:func storageSnapshotKey(accountHash, storageHash common.Hash) []byte {
 288552922  288552922    198:	return append(append(SnapshotStoragePrefix, accountHash.Bytes()...), storageHash.Bytes()...)
         .          .    199:}
         .          .    200:
         .          .    201:// storageSnapshotsKey = SnapshotStoragePrefix + account hash + storage hash
         .          .    202:func storageSnapshotsKey(accountHash common.Hash) []byte {
         .          .    203:	return append(SnapshotStoragePrefix, accountHash.Bytes()...)
ROUTINE ======================== github.com/ethereum/go-ethereum/core/rawdb.storageTrieNodeKey in github.com/ethereum/go-ethereum/core/rawdb/schema.go
 193875095  193875095 (flat, cum)  1.99% of Total
         .          .    261:func storageTrieNodeKey(accountHash common.Hash, path []byte) []byte {
 193875095  193875095    262:	return append(append(trieNodeStoragePrefix, accountHash.Bytes()...), path...)
         .          .    263:}
         .          .    264:
         .          .    265:// IsLegacyTrieNode reports whether a provided database entry is a legacy trie
         .          .    266:// node. The characteristics of legacy trie node are:
         .          .    267:// - the key length is 32 bytes

This PR reduces the number of allocs, and increases the speed of creating these keys

Benchmarks (the suffix 2 denotes the new ones):

BenchmarkStorageSnapshot-24    	14854363	        99.08 ns/op	     144 B/op	       2 allocs/op
BenchmarkStorageSnapshot2-24    	1000000000	         0.2276 ns/op	       0 B/op	       0 allocs/op
BenchmarkStorageTrieNodeKey-24    	19944460	        97.97 ns/op	     144 B/op	       2 allocs/op
BenchmarkStorageTrieNodeKey2-24    	40256310	        49.82 ns/op	      80 B/op	       1 allocs/op

@@ -188,7 +188,11 @@ func accountSnapshotKey(hash common.Hash) []byte {

// storageSnapshotKey = SnapshotStoragePrefix + account hash + storage hash
func storageSnapshotKey(accountHash, storageHash common.Hash) []byte {
return append(append(SnapshotStoragePrefix, accountHash.Bytes()...), storageHash.Bytes()...)
res := make([]byte, 1+len(accountHash)*2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
res := make([]byte, 1+len(accountHash)*2)
res := make([]byte, len(SnapshotStoragePrefix)+common.HashLength*2)

@@ -247,7 +251,11 @@ func accountTrieNodeKey(path []byte) []byte {

// storageTrieNodeKey = trieNodeStoragePrefix + accountHash + nodePath.
func storageTrieNodeKey(accountHash common.Hash, path []byte) []byte {
return append(append(trieNodeStoragePrefix, accountHash.Bytes()...), path...)
res := make([]byte, 1+len(accountHash)+len(path))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
res := make([]byte, 1+len(accountHash)+len(path))
res := make([]byte, len(trieNodeStoragePrefix)+common.HashLength+len(path))

@@ -247,7 +251,11 @@ func accountTrieNodeKey(path []byte) []byte {

// storageTrieNodeKey = trieNodeStoragePrefix + accountHash + nodePath.
func storageTrieNodeKey(accountHash common.Hash, path []byte) []byte {
return append(append(trieNodeStoragePrefix, accountHash.Bytes()...), path...)
res := make([]byte, 1+len(accountHash)+len(path))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
res := make([]byte, 1+len(accountHash)+len(path))
buf := make([]byte, len(trieNodeStoragePrefix)+common.HashLength+len(path))
n := copy(buf, trieNodeStoragePrefix)
n += copy(buf[n:], accountHash.Bytes())
copy(buf[n:], path)
return buf

could be something like this

Comment on lines +191 to +195
buf := make([]byte, len(SnapshotStoragePrefix)+common.HashLength+common.HashLength)
n := copy(buf, SnapshotStoragePrefix)
n += copy(buf[n:], accountHash.Bytes())
copy(buf[n:], storageHash.Bytes())
return buf
Copy link
Contributor

@holiman holiman Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that a more minimal change would have the same effect

Suggested change
buf := make([]byte, len(SnapshotStoragePrefix)+common.HashLength+common.HashLength)
n := copy(buf, SnapshotStoragePrefix)
n += copy(buf[n:], accountHash.Bytes())
copy(buf[n:], storageHash.Bytes())
return buf
buf := make([]byte, 0, len(SnapshotStoragePrefix)+common.HashLength+common.HashLength)
return append(append(append(buf, SnapshotStoragePrefix...), accountHash.Bytes()...), storageHash.Bytes()...)

It is slightly less complex since it relies on the compiler instead of fiddling with n.

@fjl fjl changed the title core/rawdb: reduce allocations core/rawdb: allocate database keys with explicit size to avoid slice growth Aug 23, 2023
@fjl fjl added this to the 1.13.0 milestone Aug 23, 2023
@fjl fjl merged commit 2f4dbb4 into ethereum:master Aug 23, 2023
1 check passed
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants