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

dependency on the alloc crate should be optional. #111

Closed
Lokathor opened this issue Nov 22, 2021 · 8 comments
Closed

dependency on the alloc crate should be optional. #111

Lokathor opened this issue Nov 22, 2021 · 8 comments

Comments

@Lokathor
Copy link
Contributor

It seems like the core decompression routines can be performed without allocating at all.

We should make the use of the alloc crate optional behind a cargo feature, though this would be a breaking release to do so.

@oyvindln
Copy link
Collaborator

Yeah that seems reasonable.

@mkeeter
Copy link
Contributor

mkeeter commented Jun 13, 2022

I've implemented this in a with-alloc branch here:

  • The with-alloc feature is enabled by default
  • When not enabled, the crate skips deflate and a few parts of inflate that require allocation

Happy to open a PR, if you're willing to bring it in!

@oyvindln
Copy link
Collaborator

oyvindln commented Jun 13, 2022

Yeah that looks good. I suspect this would be a breaking change that would require a version bump though?

Also gonna need some help to check that it works fine with rustc-dep-of-std but that's not critical yet (think that's still on an older version than the current one.)

@mkeeter
Copy link
Contributor

mkeeter commented Jun 13, 2022

Yeah, I wasn't totally sure about whether this would be breaking, and the docs didn't say anything about "adding a new feature and making it enabled by default".

I think it would only break in cases where people import the library with default-features = false. Doing so would be a no-op until this change, since there weren't any default features!

Anyways, I've bumped the version, added some docs, and opened a PR here.

Ironically, after testing it out, I'm probably not going to use it in my embedded system: it reduces a compressed object's size by 9.7K, but adds 11.5K of code to decompress it, for a slight net size increase 😅

@dancrossnyc
Copy link

Regardless of Matt's use case, I would love to see this integrated. :-)

@oyvindln
Copy link
Collaborator

oyvindln commented Jul 9, 2022

yeah it's been merged in, just not in a release yet as I wanted to look at a few API tweaks for a version bumps.

@dancrossnyc
Copy link

yeah it's been merged in, just not in a release yet as I wanted to look at a few API tweaks for a version bumps.

Uh, I totally didn't miss that or anything. :-)

Thanks! I'm excited for the new release.

@oyvindln
Copy link
Collaborator

Released now, would probably be doable to do the same with compression later if there is interest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants