Compress inputs #6
Conversation
compress/src/lib.rs
Outdated
| pub fn compress(input: String) -> Vec<u8> { | ||
| let input = input.into_bytes(); | ||
| let mut output = Vec::with_capacity(input.len()); | ||
| brotli::BrotliCompress(&mut input.as_slice(), &mut output, &Default::default()).unwrap(); |
There was a problem hiding this comment.
Instead of the default parameters we could probably specify a quality to get even smaller things.
**Why?** So we can compress inputs because 🤷.
Using the new `compress` member. Also ignore any future `input.txt`s.
mlodato517
force-pushed
the
compress-inputs
branch
from
94e72b8 to
af39013
Compare
compress/src/lib.rs
Outdated
| #[macro_export] | ||
| macro_rules! aoc_input { | ||
| ($path:literal) => {{ | ||
| let file = include_bytes!("../input.compressed").to_vec(); |
There was a problem hiding this comment.
you should use $path here :)
compress/src/lib.rs
Outdated
| /// Decompresses the passed byte vector into a string. | ||
| pub fn decompress(input: Vec<u8>) -> String { | ||
| let mut output = Vec::with_capacity(input.len()); | ||
| brotli::BrotliDecompress(&mut input.as_slice(), &mut output).unwrap(); |
There was a problem hiding this comment.
Mark was smarter than me and noticed this was weird. He might figure out why this needs to be this way.
There was a problem hiding this comment.
So there's a lot to say here. I'll leave high level overviews and we can dig into anything else if we feel like it!
- The reason why we need a
&muthere is not because of anything to do with theReadtrait - it's just required by theBrotliDecompressAPI. - According to the Rust API guidelines,
BrotliDecompressreally should accept readers and writers by value, not by&mut. However making the switch is a breaking change! First, to see why this is surprising, look at this example showing it "isn't" a breaking change. Next, look at this example showing it is a breaking change! What's going on here?? It turns out to be a fairly complicated topic calledreborrowing. It doesn't work trivially with generics so there's a bit of tension here between the Rust API guidelines recommending something for flexibility while that flexibility also requires surprise manual reborrowing. Either way, we can't just make a PR to update it :-)
There was a problem hiding this comment.
Though I did take this opportunity to rewrite the compress library in terms of Readers and Writers though I'm not sure it's actually better - 5599a7f. One benefit is that the compress binary doesn't need to read into strings and write out to files - it reads from a file and writes directly to a file 🤷
There was a problem hiding this comment.
Oh and another thing that was originally weird - we wondered why the std::io::Read trait took &mut self all the time since reading sounded like something you could do in a shared setting. I imagine this is because reading from a network socket or file requires updating internal state that's unsafe to do with a shared reference.
**This Commit** Updates the `aoc_input` macro to use the passed `$path` when determining which file to read. **Why?** Because the original was definitely a typo. However, we could decide that all inputs are in a consistent place and not have the macro take an argument but this seems less weird for now. See #6 (comment) for more details.
**Why?** For fun.
|
I think we've learned some cool stuff here about the |
This PR
day1(for a saving of like 87%!)day1to use the compressed inputinput.txtsWhy?
I don't know, saving space sounded nice and it wasn't
something I'd ever tried before. The
brotlilibrary has prettyrough struct/function names and was missing a lot of docs
so that could be a nice contribution sometime!