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

trie: reduce allocs in recHash #27770

Merged
merged 2 commits into from
Aug 18, 2023
Merged

Conversation

MariusVanDerWijden
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden commented Jul 25, 2023

Reduces allocations in recHash.
I noticed during profiling, that a lot of allocations (3.8% of total) is spent on hexToCompact. This uses the existing HexToCompactInPlace to reduce allocations.
Since we reset the key anyway after this operation, it should be fine imo

ROUTINE ======================== github.com/ethereum/go-ethereum/trie.keybytesToHex in github.com/ethereum/go-ethereum/trie/encoding.go
 150455888  150455888 (flat, cum)  3.22% of Total
         .          .     97:func keybytesToHex(str []byte) []byte {
         .          .     98:    l := len(str)*2 + 1
 150455888  150455888     99:    var nibbles = make([]byte, l)
         .          .    100:    for i, b := range str {
         .          .    101:        nibbles[i*2] = b / 16
         .          .    102:        nibbles[i*2+1] = b % 16
         .          .    103:    }
         .          .    104:    nibbles[l-1] = 16
(pprof) list hexTo    
Total: 4677552141
ROUTINE ======================== github.com/ethereum/go-ethereum/trie.hexToCompact in github.com/ethereum/go-ethereum/trie/encoding.go
 178393826  178393826 (flat, cum)  3.81% of Total
         .          .     37:func hexToCompact(hex []byte) []byte {
         .          .     38:    terminator := byte(0)
         .          .     39:    if hasTerm(hex) {
         .          .     40:        terminator = 1
         .          .     41:        hex = hex[:len(hex)-1]
         .          .     42:    }
 178393826  178393826     43:    buf := make([]byte, len(hex)/2+1)
         .          .     44:    buf[0] = terminator << 5 // the flag byte
         .          .     45:    if len(hex)&1 == 1 {
         .          .     46:        buf[0] |= 1 << 4 // odd flag
         .          .     47:        buf[0] |= hex[0] // first nibble is contained in the first byte
         .          .     48:        hex = hex[1:]

Benchmarks:

BenchmarkHexToCompact-24    	94361824	        21.67 ns/op	       4 B/op	       1 allocs/op
BenchmarkHexToCompactInPlace-24    	285524278	         4.462 ns/op	       0 B/op	       0 allocs/op

@rjl493456442 rjl493456442 self-requested a review July 25, 2023 12:02
@rjl493456442 rjl493456442 self-assigned this Jul 25, 2023
trie/encoding.go Outdated Show resolved Hide resolved
@holiman
Copy link
Contributor

holiman commented Jul 27, 2023

Since we reset the key anyway after this operation, it should be fine imo

Please elaborate a bit, it's not obvious to me. What reset?

@MariusVanDerWijden
Copy link
Member Author

In the stacktrie, we reset the key with st.key = st.key[:0]

@fjl fjl merged commit 5976e58 into ethereum:master Aug 18, 2023
1 check passed
@fjl fjl added this to the 1.13.0 milestone Aug 18, 2023
@fjl
Copy link
Contributor

fjl commented Aug 18, 2023

Very cool find.

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
sduchesneau pushed a commit to streamingfast/go-ethereum that referenced this pull request Dec 19, 2023
# Conflicts:
#	trie/stacktrie.go
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