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

use libdeflate rather than zlib #769

Merged
merged 3 commits into from
Oct 13, 2024
Merged

Conversation

cldellow
Copy link
Contributor

@cldellow cldellow commented Oct 6, 2024

The zlib implementation that ships with most distributions is fairly slow, even when you allow for the zlib algorithm itself being quite slow (vs lz4 and zstd).

There are faster implementations. libdeflate 1 is one such example. According to benchmarks 2, it's maybe 2-3x faster than zlib.

This PR updates helper.cpp's compression routines to use libdeflate.

It saves ~2-3% of total execution time for me:

Creating a GB mbtiles (zlib):

real 2m1.706s
user 28m24.186s
sys 0m41.886s

Creating a GB mbtiles (libdeflate):

real 1m58.450s
user 27m32.579s
sys 0m51.848s

Snapshotting an external dependency is sorta distasteful - I worry about how much is too much from a maintenance POV. To mitigate that, the snapshot is a direct copy of the upstream folders, so it should be easy to update in the future if needed.

This also lets us drop the zlib1g-devel and boost-iostreams dependencies, which is nice.

cldellow and others added 3 commits October 6, 2024 13:50
The zlib implementation that ships with most distributions is fairly slow,
even when you allow for the zlib algorithm itself being quite slow (vs
lz4 and zstd).

There are faster implementations. `libdeflate` [1] is one such example.
According to benchmarks [2], it's maybe 2-3x faster than zlib.

This PR updates helper.cpp's compression routines to use libdeflate.

It saves ~2-3% of total execution time for me:

Creating a GB mbtiles (zlib):

```
real 2m1.706s
user 28m24.186s
sys 0m41.886s
```

Creating a GB mbtiles (libdeflate):

```
real 1m58.450s
user 27m32.579s
sys 0m51.848s
```

Snapshotting an external dependency is sorta distasteful - I worry
about how much is too much from a maintenance POV. To mitigate that,
the snapshot is a direct copy of the upstream folders, so it should
be easy to update in the future if needed.

This also lets us drop the zlib1g-devel and boost-iostreams
dependencies, which is nice.

[1]: https://github.com/ebiggers/libdeflate
[2]: zlib-ng/zlib-ng#1486
Oops: if you ever passed a non-default compression level, we'd
try to create a new compressor. But because we were using the
default constructors, we'd happily create a temporary, copy its
pointer over to the `compressor` field, then destruct the temporary,
freeing the underlying native resource, and the world would end.

Instead, delete the default constructors and require an explicit
`setLevel` call.

Also update tests to verify that the different levels work.
@systemed
Copy link
Owner

Thanks - useful improvement! Tested on Ubuntu, Intel Mac and ARM Mac and happily compiles and runs on all three.

@systemed systemed merged commit fe8399e into systemed:master Oct 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants