search

bonsairobo / building-blocks

A voxel library for real-time applications.
MIT License
369 stars 30 forks source link

feat: Add rust-snappy as an alternative compression backend #6

Closed indiv0 closed 3 years ago

indiv0 commented 3 years ago

Hello again!

Here's the PR that adds support for rust-snappy as a compression backend. I tried to modify the existing structs and traits to make them generic over compression backends but I was in over my head with that so I went with the config-flag based approach.

indiv0 commented 3 years ago

Looks like CI failed due to an issue unrelated to the code:

 Get:77 http://azure.archive.ubuntu.com/ubuntu bionic-updates/universe amd64 libsdl2-dev amd64 2.0.8+dfsg1-1ubuntu1.18.04.4 [683 kB]
E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/s/systemd/libudev-dev_237-3ubuntu10.42_amd64.deb  404  Not Found [IP: 52.252.75.106 80]
Fetched 7747 kB in 0s (27.4 MB/s)
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
Error: Process completed with exit code 100.
bonsairobo commented 3 years ago

I hope you don't mind, but I want to play around with this a bit to see if I can make this work with generics to reduce some of the code duplication.

bonsairobo commented 3 years ago

@indiv0 I'm doing some refactoring to make it easier to support generic compression schemes. I think the changes to compressible-map are done. I've merged with those in building-blocks. Now I think I just need to replace a hardcoded Lz4 with a type parameter to support snappy.

bonsairobo commented 3 years ago

So I'm going to close this PR in favor of an upcoming patch that should accomplish the same goal.

bonsairobo commented 3 years ago

@indiv0 I just committed this patch that makes snappy the new default compression algorithm for ChunkMap.

https://github.com/bonsairobo/building-blocks/commit/cfd2f43bee1087630f71f5decd1e5ed3d0a579c3

bonsairobo commented 3 years ago

@indiv0 I actually ran some compression benchmarks on test VOX data, and saw that LZ4 was about 10x better compression, although Snappy is a bit faster. I think LZ4 has the better tradeoff, so I'm going to make that the default again. But you can still use Snappy with the "snappy" feature.

indiv0 commented 3 years ago

@bonsairobo awesome, thanks so much! I'll give it a shot as soon as I can. Interesting that the difference in compression ratio is so high. I'll play a bit with them myself.

indiv0 commented 3 years ago

@bonsairobo so I gave it a try, it seems to work well, thanks again!

Only issue is that there doesn't appear to be an easy way to disable the building_blocks_storage/lz4 feature when declaring building_blocks as a dependency. To get around this I had to maintain my fork of building_blocks and disable the feature there.

bonsairobo commented 3 years ago

@indiv0 Hey glad to hear it's mostly working.

I think I just forgot to export the lz4 feature from the top-level crate, and because "lz4" is a default feature for the "building_blocks_storage" crate, it was getting pulled in.

I just made a commit so you should be able to disable "lz4" directly from "building-blocks."