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

Musli-wire forces variable-length integer encoding everywhere inside #[musli(packed)] #23

Open
cormac-ainc opened this issue May 23, 2023 · 1 comment
Labels
enhancement New feature or request

Comments

@cormac-ainc
Copy link

I was looking into why a #[musli(packed)] struct Point3 { x: f32, y: f32, z: f32 } was so small when it was all-zeros despite setting the integer encoding to fixed length, and I discovered that ALL fields in packed structs use variable length int encoding. You can't turn it off by choosing fixed length integers in your musli-wire Encoding. Since musli-wire implements packed using musli-storage, I would have thought it would just pass the variable/fixed-ness down to that. But it doesn't:

type Encoder<'this> = StorageEncoder<&'this mut FixedBytes<P, W::Error>, Variable, Variable> where Self: 'this;
(also see the decoder using Variable, Variable).

This was introduced in 7a3c387

Of course you may not consider this a bug. Packed structs are supposed to be small, gotta save space for only 62 bytes, etc.

For making the behaviour optional, you could add type params to Encoding specifically for ints and lengths in a packed struct. To avoid breaking changes to people's formats, the type parameters could default to Variable. And then you would pass these down to the StorageEncoder/Decoder instead of always passing Variable.

@udoprog
Copy link
Owner

udoprog commented May 23, 2023

Good point, and that's not supposed to be Variable in my mind either. And the lack of configuring the packed encoding has bothered me as well.

Maybe we can make the packed encoder configurable through a builder trait, which defaults to something sensible?

Incomplete, but rough idea:

trait PackedEncoderBuilder {
    type Buf;
    type Encoder<'buf>;

    fn build<'buf>(buf: &'buf mut Self::Buf) -> Self::Encoder<'buf>;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants