-
Notifications
You must be signed in to change notification settings - Fork 182
Seperate trie commit and hash #1656
Seperate trie commit and hash #1656
Conversation
…perate-trie-commit-and-hash
Benchmark Hash and commit
|
The reason that the execution time of the hash function is longer than expected is because parallel processing is removed from this part of code. This was rolled back in PR #1659, and you can see that the performance improvement occurred in that PR. |
// For children in range [0, 15], it's impossible | ||
// to contain valueNode. Only check the 17th child. | ||
if n.Children[16] != nil { | ||
c.onleaf(nil, nil, n.Children[16].(valueNode), hash, 0) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about this? Would you mind to take a look insert
method of Trie
?
Please let me know if I'm wrong.
_, branch.Children[n.Key[matchlen]], err = t.insert(nil, append(prefix, n.Key[:matchlen+1]...), n.Key[matchlen+1:], n.Val)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think there's no specific problem in here.
Same as the existing onleaf method.
@kjhman21 Was this PR milestone changed to v1.11? |
@hqjang-pepper yes. |
Won't do in this version. closing. |
Proposed changes
Seperate Trie committer and hasher
Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test
)Related issues
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...