Improve cluster plotting GPU utilization #3262
Merged
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
teor2345
previously approved these changes
Nov 26, 2024
| // Allow to buffer up to the whole sector in memory to not block plotter on the | ||
| // other side | ||
| let (mut sector_sender, sector_receiver) = mpsc::channel( | ||
| sector_size(pieces_in_sector) / nats_client.approximate_max_message_size(), |
There was a problem hiding this comment.
Is there an edge case here where the sector size is very small, and the message size is very large?
I didn’t do the calculations in detail, but I think that we can get a situation where sector size is around 1 MB, and message size is 2 MB? Which would panic due to a zero-sized channel.
If the minimum number of pieces per sector is high enough, and the upper limit on the message size is low enough, this won’t be a problem, but I’m not familiar enough with this code to know.
There was a problem hiding this comment.
It is unlikely, but possible. Good catch, thanks!
shamil-gadelshin
approved these changes
Nov 27, 2024
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.
This allows farmer to buffer the whole sector in memory in case disk is not fast enough to write, which will release plotting resources on the other end and allow plotter to start plotting other sectors, potentially for a completely different machine.
Code contributor checklist: