Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.

feat: use faster isal when available #6

Merged
merged 8 commits into from
Dec 29, 2023
Merged

feat: use faster isal when available #6

merged 8 commits into from
Dec 29, 2023

Conversation

bdraco
Copy link
Owner

@bdraco bdraco commented Dec 29, 2023

isal recommended for speed by pycompression authors pycompression/python-zlib-ng#24 (comment)

Tested on x86_64, macos arm64, and aarch64

Copy link

codecov bot commented Dec 29, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (6b57e33) 77.77% compared to head (6ab36b6) 71.42%.
Report is 1 commits behind head on main.

Files Patch % Lines
src/aiohttp_zlib_ng/__init__.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #6      +/-   ##
==========================================
- Coverage   77.77%   71.42%   -6.35%     
==========================================
  Files           1        1              
  Lines          18       21       +3     
  Branches        4        4              
==========================================
+ Hits           14       15       +1     
- Misses          2        4       +2     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bdraco bdraco marked this pull request as ready for review December 29, 2023 23:03
@bdraco bdraco merged commit ecdaf7d into main Dec 29, 2023
19 of 21 checks passed
@bdraco bdraco deleted the isal_when_available branch December 29, 2023 23:03
@rhpvorderman
Copy link

As a general comment: isa-l compresses to much lower compression levels by default. In case that is undesirable, it is better to use zlib-ng.

On the other hand, if deflate compression is bottlenecking, shaving another 20% of the filesize is not really worth the delay in latency sensitive situations. The difference in speed between zlib-ng level 2 and isa-l level 2 will be much less than the difference between isa-l level 2 (default) and zlib-ng level 6 (default).

zlib-ng level 1 is much bigger than either zlib level 1 or isa-l level 1, at least on genomic data. So I don't recommend using that.

@bdraco
Copy link
Owner Author

bdraco commented Dec 31, 2023

Seems to be a better trade off with using isal. It's faster and we get better compression for the websocket use case

@rhpvorderman
Copy link

Then they are probably already using level 1. Then ISA-L is so much better. The compression speed is incredible, almost as fast as the decompression speed, 5 times faster than zlib. It really is an incredible library. I prefer using ISA-L where I can, and I maintain python-zlib-ng only because python-isal cannot cover all use cases. So all the cool innovations happen in python-isal first and are then ported over later when they proved stable.

@rhpvorderman
Copy link

I wonder, have you considered porting this work over to aiohttp directly? In that way everyone will benefit from your work.

@bdraco
Copy link
Owner Author

bdraco commented Jan 2, 2024

That's the eventual plan.

Usually I try out changes in libraries for a few months before upstreaming them into aiohttp.

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

Successfully merging this pull request may close these issues.

2 participants