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

Several improvements around Bytesable and Message. #601

Merged
merged 4 commits into from
Nov 24, 2024

Conversation

frankmcsherry
Copy link
Member

@frankmcsherry frankmcsherry commented Nov 24, 2024

These commits do some likely uncontroversial tidying, and then the somewhat controversial removal of the Bundle type which communicates a preference for using bincode on Message. Although we started to generalize the allowed serialization strategies, Bundle has been in the public interface for ParallelizationContract, and generally there needs to be some structure to allow the various helpers to pull out timestamps, etc. This shift does not remove flexibility that actually existed, as far as I can tell.

The intended direction is to replace the Bytesable implementation for Message, which in this PR looks like so

impl<T, C> crate::communication::Bytesable for Message<T, C>
where
    T: Serialize + for<'a> Deserialize<'a>,
    C: Serialize + for<'a> Deserialize<'a>,
{
...

with one for which we have only C: Bytesable. This allows the container to specify its strategy for serialization. There are a few ways to do this, and I haven't figured out which one is best or worst or .. whatever. That part being tbd, I thought I'd put this up for review in the meantime.

@frankmcsherry frankmcsherry merged commit 0062c29 into TimelyDataflow:master Nov 24, 2024
7 checks passed
@github-actions github-actions bot mentioned this pull request Nov 24, 2024
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.

1 participant