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

Additional rkyv support by trying to derive the Archived type as Self where possible #762

Closed
wants to merge 1 commit into from

Conversation

yongqli
Copy link

@yongqli yongqli commented Aug 9, 2022

The current support for rkyv in chrono isn't very useful, because rkyv generates a new type on each invocation of derive(Archive). This new type won't have any traits or methods implemented on it except Deserialize, so it can't be easily used in a zero-copy manner.

However, most chrono types are Copy, which means that rkyv can be configured to use Self for derive(Archive) instead of generating a new one. One downside is that this doesn't work if rkyv is configured to use a specific endianness.

Code can be generic to which specific type rkyv generated for the Archive impl through the use of rkyv::Archived<_>, so chrono can opportunistically generate a different type without breaking code that wants to support different endianness, which is also what rkyv currently does for primitives: https://docs.rs/rkyv/0.7.39/src/rkyv/impls/core/primitive.rs.html#107

@yongqli
Copy link
Author

yongqli commented Aug 9, 2022

Looks like the test runner tried to enable incompatible features.

@djc
Copy link
Member

djc commented Aug 9, 2022

Okay, that's a good reason this won't work: Cargo features must be additive and should not be mutually exclusive. Otherwise one library depending on Cargo could select one feature, another library selects the other, and the application wanting to use both libraries no longer works. So I don't think this can be merged as is.

@yongqli
Copy link
Author

yongqli commented Aug 9, 2022

One possibility is to add a feature flag to enable attempting to derive using Self instead of the negative selection as it is now. I can also contact the rkyv author for recommendations and feedback.

@esheppa
Copy link
Collaborator

esheppa commented Aug 9, 2022

rust-decimal recently resolved a similar issue to this with serde by using #[serde(with = "")] attributes which are enabled by features, which enables multiple users of the same library to serialize differently and additively. Potentially this can be handled on the consumer side by using an equivalent attribute from rkyv?

@pitdicker
Copy link
Collaborator

I agree it would be cool if Copy types would not even need a serialize/deserialize step when used with rkyv. And of course this breaks down when not everything uses the native endianness.

Closing for now. But if someone figures out a nice solution in rkyv/rkyv#292 feel free to open a new PR.

@pitdicker pitdicker closed this Sep 19, 2023
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.

4 participants