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

Variable length encoding for enum discriminants #157

Closed
dtolnay opened this issue Apr 28, 2017 · 12 comments
Closed

Variable length encoding for enum discriminants #157

dtolnay opened this issue Apr 28, 2017 · 12 comments

Comments

@dtolnay
Copy link
Collaborator

@dtolnay dtolnay commented Apr 28, 2017

I think the majority of enum discriminants are less than 128. A very simple variable length encoding like encode <128 as one byte and >=128 as four bytes would address most concerns from people who are astonished that Bincode encodes their 2-variant enum with [0x00, 0x00, 0x00, 0x00] and [0x00, 0x00, 0x00, 0x01] tags.

@dtolnay
Copy link
Collaborator Author

@dtolnay dtolnay commented Apr 28, 2017

Regarding backward compatibility - I think it would be sufficient to provide an external bincode_upgrade crate that takes a &[u8] of old data and a type T and gives you a Vec<u8> of new data.

@arthurprs
Copy link

@arthurprs arthurprs commented Apr 28, 2017

IMHO there's little to no benefit in creating yet another varint variant. Most will fit 0-127 in a single byte and they could be just reused for sequences/map/string lengths.

@dtolnay
Copy link
Collaborator Author

@dtolnay dtolnay commented Apr 28, 2017

Fair enough, let's do that.

@TyOverby
Copy link
Collaborator

@TyOverby TyOverby commented Apr 28, 2017

LEB128 is a pretty nice format (used in the DWARF format I think) that has a solid Rust crate.

@dtolnay
Copy link
Collaborator Author

@dtolnay dtolnay commented May 12, 2017

I filed #182 to provide this in an opt-in way for integer types as well.

@rnewman
Copy link

@rnewman rnewman commented Feb 7, 2018

Is there consensus on making this change? I just ran into this, and I'd rather have bincode do all the work than manually serialize my enum's tag to save 3 bytes per value.

@arthurprs
Copy link

@arthurprs arthurprs commented Feb 16, 2018

It might be nice to consider (benchmark) PrefixVarint in addition to LEB128.

@cynecx
Copy link

@cynecx cynecx commented Sep 9, 2018

Has there been any plans about implementing this? I am really interested in this, as it could even further optimize my use-cases.

Actually I've been playing around replacing the variant-index with a le-prefix-varint: master...cynecx:prefix-varint (The code is really just hacky, I was confused by all the serde api stuff which I am not familiar with).

@maciejhirsz
Copy link
Contributor

@maciejhirsz maciejhirsz commented Jun 7, 2019

@dtolnay @TyOverby what about adding a varint (or some such) feature to the bincode crate that, when turned on, will treat enum discriminants and sequence lengths as varints? That should be pretty easy to do I reckon, and wouldn't break any existing code.

I'd be happy to pen a PR for this.

@jdm
Copy link
Member

@jdm jdm commented Jun 7, 2019

@maciejhirsz That sounds like a useful path forward. We would be willing to merge that pull request.

@tkaitchuck
Copy link

@tkaitchuck tkaitchuck commented Dec 13, 2019

I think #294 could serve as a reasonable template for this.
Similar to how it adds support for configuring lengths of arrays and strings, a value could be added for enums.

@ZoeyR
Copy link
Collaborator

@ZoeyR ZoeyR commented Apr 16, 2020

Closing in favor of #319

@ZoeyR ZoeyR closed this Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

9 participants
You can’t perform that action at this time.