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

turbohash #171

Merged
merged 1 commit into from
Dec 19, 2024
Merged

turbohash #171

merged 1 commit into from
Dec 19, 2024

Conversation

suurkivi
Copy link
Collaborator

@suurkivi suurkivi commented Dec 17, 2024

This change stores the hash of a trie node in the node's parent (in a hashmap).

This yields an important performance improvement when recomputing the hash of a changed node. Previously, when recomputing the hash, as we are working our way up the trie, we had to load siblings of a changed node out of the db. In this version, the sibling hashes are already loaded, which dramatically reduces the number of db reads.

assert_eq!(r.unwrap()[0], true);
assert_eq!(node.items(), 0);
assert_eq!(node.hash(), empty_hash());
assert_eq!(node.hash(), Vec::<u8>::new());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note a subtle change here

assert_eq!(r.unwrap()[0], true);
assert_eq!(node.items(), 0);
assert_eq!(node.hash(), empty_hash());
assert_eq!(node.hash(), Vec::<u8>::new());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note a subtle change here

@@ -122,15 +124,25 @@ impl TrieNode {
for char in db_trie_node.child_chars {
children.insert(
char as u8,
TrieNodeType::Serialized(SerializedTrieNode::new(None)),
TrieNodeType::Serialized(SerializedTrieNode::new()),
Copy link
Contributor

Choose a reason for hiding this comment

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

If the struct is empty, do we actually need the struct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

left a TODO comment

@suurkivi suurkivi force-pushed the js/turbohash branch 3 times, most recently from 22bb674 to d0d04ba Compare December 19, 2024 19:56
@suurkivi suurkivi merged commit 03ebb7b into main Dec 19, 2024
2 checks passed
@suurkivi suurkivi deleted the js/turbohash branch December 19, 2024 21:40
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.

2 participants