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.

Sign up for GitHub

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

Jump to bottom

trie: small optimization of delete in fullNode case #22979

Merged
merged 3 commits into from Jun 20, 2021
Merged

trie: small optimization of delete in fullNode case #22979

merged 3 commits into from Jun 20, 2021

Conversation

Evolution404
Copy link
Contributor

when nn is not nil, there is no need to check the number of non-nil entries.
Since n must've contained at least two children before deletion, there must be another child node other than nn.

@holiman
Copy link
Contributor

holiman commented Jun 1, 2021

I see you opened a separate PR instead of #22960. Next time, just make another commit on top. We squash the commits into one when merging anyway.
If you want, you can squash it yourself

git commit -m "foo bar"
git rebase -i HEAD~2
# Then select the latest for squashing into the previous
# and finally, since it's squashed, you need to force-push
git push -f

@Evolution404 Evolution404 reopened this Jun 1, 2021
@Evolution404
Copy link
Contributor Author

Thanks for reminding

fjl
fjl reviewed Jun 18, 2021
View reviewed changes
trie/trie.go Show resolved Hide resolved
@fjl fjl removed the status:triage label Jun 18, 2021
@fjl fjl added this to the 1.10.5 milestone Jun 18, 2021
fjl
fjl approved these changes Jun 18, 2021
View reviewed changes
Copy link
Contributor

@fjl fjl left a comment

Choose a reason for hiding this comment

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

Very nice!

@fjl fjl changed the title trie/trie.go improve the delete performance trie: improve performance of delete in fullNode case Jun 18, 2021
@fjl fjl changed the title trie: improve performance of delete in fullNode case trie: small optimization of delete in fullNode case Jun 20, 2021
@fjl fjl merged commit 732a6a3 into ethereum:master Jun 20, 2021
reds pushed a commit to reds/go-ethereum that referenced this pull request Aug 28, 2021
@Evolution404 @fjl
trie: small optimization of delete in fullNode case (ethereum#22979)
e26eba7 
When deleting in fullNode, and the new child node nn is not nil, there is no need
to check the number of non-nil entries in the node. This is because the fullNode 
must've contained at least two children before deletion, so there must be another
child node other than nn.

Co-authored-by: Felix Lange <fjl@twurst.com>
i-norden pushed a commit to cerc-io/go-ethereum that referenced this pull request Sep 10, 2021
@Evolution404 @fjl @i-norden
trie: small optimization of delete in fullNode case (ethereum#22979)
00b2cd0 
When deleting in fullNode, and the new child node nn is not nil, there is no need
to check the number of non-nil entries in the node. This is because the fullNode 
must've contained at least two children before deletion, so there must be another
child node other than nn.

Co-authored-by: Felix Lange <fjl@twurst.com>
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
@Evolution404 @fjl @atif-konasl
trie: small optimization of delete in fullNode case (ethereum#22979)
80f8e62 
When deleting in fullNode, and the new child node nn is not nil, there is no need
to check the number of non-nil entries in the node. This is because the fullNode 
must've contained at least two children before deletion, so there must be another
child node other than nn.

Co-authored-by: Felix Lange <fjl@twurst.com>
This was referenced Sep 23, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjl fjl fjl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.5
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Evolution404 @holiman @fjl