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

Rkuris/nibbles iterative improvement #220

Merged
merged 8 commits into from
Aug 25, 2023

Conversation

rkuris
Copy link
Collaborator

@rkuris rkuris commented Aug 25, 2023

Nibbles: impl FusedIterator + DoubleEndedIterator

I need DoubleEndedIterator for a later diff, but I found that implementing these interfaces forced some optimization of Nibbles.

Implenting DoubleEndedIterator improved performance by precomputing the
constant maximum index offset and storing it. This means the Iterator
type is 4 bytes larger, but this seems like a good tradeoff.

Implementing `FusedIterator` also improved performance in two ways:
 - Calling `next()` does not increment pos if you're already at the end
 - The `is_empty()` check is now an equals rather than a greater or
   equal

But these implementations have some great benefits:
 - Additional methods like rev() are now available to iterate backwards
 - `fused()` on the iterator is now a no-op

Attempting to get ahead of future stable lint snags.

This one found a few inefficient memory initializations and some useless
statements.
Per review comments
I need DoubleEndedIterator for a later diff, but I found that
implementing these interfaces forced some optimization of Nibbles.

Implenting DoubleEndedIterator improved performance by precomputing the
constant maximum index offset and storing it. This means the Iterator
type is 4 bytes larger, but this seems like a good tradeoff.

Implementing `FusedIterator` also improved performance in two ways:
 - Calling `next()` does not increment pos if you're already at the end
 - The `is_empty()` check is now an equals rather than a greater or
   equal

But these implementations have some great benefits:
 - Additional methods like rev() are now available to iterate backwards
 - `fused()` on the iterator is now a no-op
@rkuris rkuris marked this pull request as ready for review August 25, 2023 17:00
@richardpringle richardpringle merged commit 7e9dc91 into main Aug 25, 2023
5 checks passed
@richardpringle richardpringle deleted the rkuris/nibbles-iterative-improvement branch August 25, 2023 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants