-
Notifications
You must be signed in to change notification settings - Fork 7
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
Stop defaulting to using enums as deltas, use bitflagged structs with custom Serialize and Deserialize instead #2
Comments
Maybe a downside worth mentioning is that the I've been looking for a crate like this for some time, even started to work on one myself. I'll just use yours :) |
That's a good point. There are already some compile time errors when you have too many fields for the enum based deltas such as: dipa/crates/dipa-derive-test/src/all_tests/ui/max_delta_batch_too_large.stderr Lines 1 to 7 in adf0586
So yeah we'd have documentation and also a compile time error that tells you to use |
Actually sorry - there wouldn't be a field limit. We are totally free to use multiple For example, if there are 22 fields we would use a Or, if it made the logic simpler, we may just use only |
Maybe I'm misunderstanding something here but wouldn't you need only 3 |
For the 22 field example? You need to be able to say which of the 22 fields are included in the serialized format. This would require 22 bits of information. A single But 1 u8 would be totally fine for structs with 8 or fewer fields. Hopefully that makes sense. |
Yes, of course. |
Right now if a struct has a small number of fields we default to using an enum to encode the delta.
Something like:
The idea behind this was to be able to only transmit exactly what has changed at the cost of a single byte of overhead (the enum variant).
The downside of the enum approach is that it can't support more than a few fields. If you had 10 fields you would more than 2^10 (1024) enum variants to encode every combination of fields changing.
There is also some code complexity in needing to generate diffing and patching functions that have match blocks that match on every possible variant.
We can accomplish the 1 byte overhead without the downsides of the enum approach.
Something like:
We keep a single
u8
within which we can use the 8 bits to encode whether or not up to 8 fields in the struct have changed.If a struct has more than 8 fields we can use a
u16
to encode changes.Technically the enum approach would only need 1 byte overhead even if you have more than 8 fields, but I suspect that compile times would begin to suffer. This needs some investigation so that we can gauge the impact on compile times. We can leave the enum approach around as a non default approach and document our findings on the compile time impact.
From there we would generate a custom Serialize and Deserialize implementation for
MyStructDelta
. If a field isOption::None
, we skip it when serializing the struct to a map.When deserializing, we look at the
changed_fields
bits to know which fields to deserialize the map to.Implementation
We would add a new
FieldBatchingStrategy
(probably needs to be renamed to something that better communicates that all it is is the way that we want to generate the delta. PerhapsDeltaFormat
) variant, perhapsSelf::BitflaggedStruct
. We'd then add new tests and code todipa-derive
anddipa-derive-test
in order to support this new way of generating deltas.Then we'd update the documentation to explain the format and why it is the default.
The text was updated successfully, but these errors were encountered: