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

Align Bytesable messages to u64 #614

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

frankmcsherry
Copy link
Member

The Bytesable trait is what indicates how we want to serialize messages, should we need to do that. There is an advantage to data containers being u64 aligned, specifically for columnar implementations but potentially for other implementations. This PR adds up to 7 bytes for each message to ensure that each part ends up u64 aligned. With these changes, the columnar example does not need to relocate (memcpy) in order to get aligned data.

Tbd: other containers (e.g. Vec, FlatStack) don't have the same defensive measures in place yet, and we'll want to figure out how to prevent them from messing anything up either. Their ContainerBytes implementation could do this, or the Bytesable implementation for Message could round up once it has serialized everything.

Comment on lines +111 to +112
pub trait ExchangeData: Data + encoding::Data + columnar::Columnar { }
impl<T: Data + encoding::Data + columnar::Columnar> ExchangeData for T { }
Copy link
Member Author

Choose a reason for hiding this comment

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

This .. worked out ok but I realized is from a prior attempt that wanted to columnarize things. Unnecessary for this PR.

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