Skip to content

trie: avoid rlp-encoding when trie hash is already available#20461

Closed
holiman wants to merge 2 commits intoethereum:masterfrom
holiman:trie_fixes
Closed

trie: avoid rlp-encoding when trie hash is already available#20461
holiman wants to merge 2 commits intoethereum:masterfrom
holiman:trie_fixes

Conversation

@holiman
Copy link
Copy Markdown
Contributor

@holiman holiman commented Dec 16, 2019

I'm not sure this is correct, of if I'm missing some quirky edgecase, but what this PR does, is attempt to improve on the scenario where we first Hash the trie and later Commit it.

That typically happens on every block, since we call IntermediateStateRoot after applying the block rewards, and that does Hash internally. We need to hash it to get the state root, and we need that in order to validate against the header.
If everything checks out, we then Commit. Now, the Commit is clever enough not to re-hash everything since it already has the hashes cached for all nodes. However, we do rlp-encode everything the second time around, which this PR skips if we have the hash cached.

Turns out that saves us 20% processing time:

[user@work trie]$ benchstat before.txt after.txt 
name               old time/op    new time/op    delta
CommitAfterHash-6    5.08µs ±12%    3.99µs ±21%  -21.33%  (p=0.000 n=20+18)

name               old alloc/op   new alloc/op   delta
CommitAfterHash-6    1.79kB ± 1%    1.65kB ± 1%   -7.67%  (p=0.000 n=18+17)

name               old allocs/op  new allocs/op  delta
CommitAfterHash-6      11.0 ± 0%      10.0 ± 0%   -9.09%  (p=0.000 n=20+20)

@holiman holiman changed the title Trie fixes avoid rlp-encoding when trie hash is already available Dec 16, 2019
@holiman holiman changed the title avoid rlp-encoding when trie hash is already available trie: avoid rlp-encoding when trie hash is already available Dec 16, 2019
Comment thread trie/hasher.go
if len(h.tmp) < 32 && !force {
return n, nil // Nodes smaller than 32 bytes are stored inside their parent
}
// Larger nodes are replaced by their hash and stored in the database.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this doesn't work, the code below assumes h.tmp contains the rlp-encoding of the node :(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But actually, the db.insert only cares about the size (for tracking), not the actual data

@holiman holiman closed this Jan 4, 2020
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.

1 participant