Implement intra_rebase to reuse identical subtrees
#65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michaelsproul
changed the title
intra_rebase for self-similar listsintra_rebase to reuse identical subtrees
michaelsproul
commented
Mar 3, 2025
| if let Some(known_subtree) = known_subtrees.get(&(current_depth, hash)) { | ||
| // Node is already known from elsewhere in the tree. We can replace it without | ||
| // looking at further subtrees. | ||
| return Ok(IntraRebaseAction::Replace(known_subtree.clone())); |
There was a problem hiding this comment.
A small further optimisation here would be to avoid doing a Replace if the known_subtree and orig are already ptr-equal, e.g. if we call intra_rebase twice in a row it should always be a Noop (we could write a test for this).
michaelsproul
commented
Mar 3, 2025
| current_depth: usize, | ||
| ) -> Result<IntraRebaseAction<Self>, Error> { | ||
| match &**orig { | ||
| Self::Leaf(_) | Self::PackedLeaf(_) | Self::Zero(_) => Ok(IntraRebaseAction::Noop), |
There was a problem hiding this comment.
Another optimisation would be to save some stack depth by avoiding recursing down to the leaves. Once we reached a specific depth above the leaves we could just stop recursing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While debugging high memory usage on Holesky, we found that the changing
inactivity_scoreswere occupying a large amount of RAM: around 70MB of new data each epoch boundary. See:With this change, we can reduce it down to 4MB, with a rebase op taking only 42ms.
The
intra_rebaseimplementation is a recursive depth-first tree traversal. It's similar torebase, but a bit simpler.