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

Add PieceCache::read_pieces() method for more efficient piece cache reads in batches #3147

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

nazar-pc
Copy link
Member

@nazar-pc nazar-pc commented Oct 18, 2024

This builds on #3135 and #3141 to implement more efficient piece reading.

For local cache we now have a pool of open files. DirectIoFile has mutex internally due to need to manage its read/write buffer for aligned I/O and without pool concurrent reads are not actually happening despite &self of the FileExt trait it implements. Also it is from our experience with farmer faster to read from multiple handles in general, especially on Windows (though due to memory usage issues Windows is still doing blocking non-concurrent reads).

For cluster cache we now have stream request for faster retrieval of pieces, which was much easier to implement after refactoring done in #3141

I also clarified guarantees about returned values in a few places.

The biggest remaining topic is implementing PieceGetter::get_pieces() for ClusterPieceGetter and remove dummy implementation from PieceGetter.

Not particularly happy with number of hashmaps required in piece cache handling, but I didn't want to force all results being returned in-order either and without guaranteed order we need an efficient way to track pieces, hence hashmaps 😞

Code contributor checklist:

Copy link
Member

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Looks good, I just had a question about a trait method invariant.

crates/subspace-farmer/src/single_disk_farm/piece_cache.rs Outdated Show resolved Hide resolved
@nazar-pc nazar-pc force-pushed the batches-in-piece-cache-api branch from 01f5c1f to 20a371f Compare October 18, 2024 08:06
@nazar-pc nazar-pc requested a review from teor2345 October 18, 2024 08:06
@nazar-pc nazar-pc enabled auto-merge October 18, 2024 09:23
@nazar-pc nazar-pc mentioned this pull request Oct 18, 2024
1 task
@nazar-pc nazar-pc added this pull request to the merge queue Oct 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 18, 2024
@nazar-pc nazar-pc merged commit ebd4a0a into main Oct 18, 2024
8 checks passed
@nazar-pc nazar-pc deleted the batches-in-piece-cache-api branch October 18, 2024 15:27
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