Skip to content

Alternative way to do manifest split without caching the splits#1498

Merged
paraseba merged 10 commits intomainfrom
push-tqnxskxkzzqy
Feb 24, 2026
Merged

Alternative way to do manifest split without caching the splits#1498
paraseba merged 10 commits intomainfrom
push-tqnxskxkzzqy

Conversation

@paraseba
Copy link
Collaborator

No description provided.

@paraseba paraseba requested a review from dcherian December 19, 2025 20:39
@paraseba paraseba marked this pull request as ready for review February 24, 2026 18:27
@paraseba paraseba requested review from dcherian and li-em February 24, 2026 18:27
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

LGTM * (honestly tests passing is pretty good indication this is correct, i spent quite some effort on them). Thanks for rewriting my shitty code @paraseba !

}
}

self.write_manifest_from_iterator(stream::iter(all_chunks_vec).err_into()).await
Copy link
Contributor

Choose a reason for hiding this comment

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

could stream here instead of realizing a Vec. Is this where you had lifetime issues?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, basically, I'd have to replace the mutable for loops by a bunch of then, filter_map and others. I tried and failed. I think what is to blame is Manifest.iter() which is one of these weird things that take self as an Arc.

We should revisit if we find problems once we go into benchmarking.

Copy link
Contributor

@li-em li-em left a comment

Choose a reason for hiding this comment

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

This is so much cleaner now, thanks Seba!

// With IC1, doing this correctly requires updating Session.splits during rebase to match the new
// parent snapshot's array shapes.
async fn test_rebase_over_resize() -> Result<(), Box<dyn Error>> {
struct YoloSolver;
Copy link
Contributor

Choose a reason for hiding this comment

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

😹

@paraseba paraseba merged commit 5a433f6 into main Feb 24, 2026
20 checks passed
@paraseba paraseba deleted the push-tqnxskxkzzqy branch February 24, 2026 20:41
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