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

refactor(debug): simplify trie updates comparison logic #13516

Conversation

bendanzhentan
Copy link
Contributor

  • Removing redundant match patterns and nested code blocks
  • Improving warning messages to show the number of mismatched entries

- Removing redundant match patterns and nested code blocks
- Improving warning messages to show the number of mismatched entries
@bendanzhentan bendanzhentan force-pushed the perf-in-memory-merkle-check-mismatches branch from 4a6ed07 to 32f9e79 Compare December 23, 2024 09:43
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

not very familiar with this, but looks easier ptal @rkrasiuk

@bendanzhentan
Copy link
Contributor Author

@rkrasiuk ping

let in_mem_updates_iter = in_memory_updates.account_nodes_ref().iter();
let incremental_updates_iter = incremental_trie_updates.account_nodes_ref().iter();

for (in_mem, incr) in in_mem_updates_iter.zip(incremental_updates_iter) {
Copy link
Member

Choose a reason for hiding this comment

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

if length of updates is not the same, some updates will be lost whereas previously we at least produced a warning if any items are left after iteration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @rkrasiuk and sorry for the late reply.
You are correct. This change will emit some updates when the length is not the same.
I will close this PR.

}
}

if !in_mem_mismatched.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

this is incorrect because you are checking the lengths of mismatched entries whereas previously we checked whether any updates have more items

Comment on lines +219 to +221
} else if !incremental_mismatched.is_empty() {
warn!(target: "reth::cli", "Incremental trie updates have {} more entries", incremental_mismatched.len());
}
Copy link
Member

Choose a reason for hiding this comment

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

this path is unreachable, because if in_mem_mismatched is empty, incremental_mismatched will be empty as well

@bendanzhentan bendanzhentan deleted the perf-in-memory-merkle-check-mismatches branch January 7, 2025 11:34
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.

3 participants