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

SMPTNG-259: Improve block chunk generation by re-using block sizes freely #829

Merged
merged 9 commits into from
Mar 4, 2024

Conversation

scottopell
Copy link
Contributor

What does this PR do?

When generating the block chunks that will be filled with a payload, instead of attempting to maintain even weightings of the block sizes, freely re-use them as needed to provide more capacity in the block chunks.

This also provides useful warnings when the payload cannot be fulfilled as written, eg:

2024-02-29T22:21:42.155929Z  WARN lading_payload::block: Failed to construct chunks adding up to 100 MB. Chunks created have total capacity of 10.24 MB. 89.76 MB unfulfilled. Max capacity of chunks was hit, consider making block sizes larger to fit more data.

Motivation

User asked for a certain amount of data, lets try harder to fulfill their request.

Related issues

Additional Notes

Note currently based on #828

@scottopell scottopell requested a review from a team as a code owner February 29, 2024 22:22
@scottopell scottopell changed the title Improve block chunk generation by re-using block sizes freely SMPTNG-259: Improve block chunk generation by re-using block sizes freely Feb 29, 2024
Comment on lines +788 to +805
// proptest todo - figure out how to express "block_byte_sizes members must be less than total_bytes"
// until then, the strategy is to set the max block size to the min total_bytes
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could write a strategy for this. https://proptest-rs.github.io/proptest/proptest/tutorial/higher-order.html. There's also https://docs.rs/proptest/latest/proptest/macro.prop_assume.html but that'll throw away a ton of cases, where if you build values that are less than total_bytes and glob them into a vec you're good.

Base automatically changed from sopell/fill-all-block-chunks to main March 1, 2024 15:20
@scottopell scottopell force-pushed the sopell/reuse-block-chunks-freely branch from 5866839 to af06b2d Compare March 1, 2024 15:47
@scottopell scottopell force-pushed the sopell/reuse-block-chunks-freely branch from 399ced4 to 29a6c8f Compare March 4, 2024 21:26
@scottopell scottopell merged commit 38d9d2a into main Mar 4, 2024
16 checks passed
@scottopell scottopell deleted the sopell/reuse-block-chunks-freely branch March 4, 2024 21:38
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.

2 participants