aio-libs / aiokafka

asyncio client for kafka
http://aiokafka.readthedocs.io/
Apache License 2.0
1.09k stars 224 forks source link

Use cramjam for LZ4 #960

Closed ods closed 5 months ago

ods commented 5 months ago

Changes

Reduces number of dependencies

Checklist

codecov[bot] commented 5 months ago

Codecov Report

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

Comparison is base (425ce26) 94.92% compared to head (c4c5cd8) 95.03%.

Files Patch % Lines
aiokafka/codec.py 66.66% 2 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #960 +/- ## ========================================== + Coverage 94.92% 95.03% +0.10% ========================================== Files 106 106 Lines 16356 16323 -33 Branches 2618 2610 -8 ========================================== - Hits 15526 15512 -14 + Misses 554 536 -18 + Partials 276 275 -1 ``` | [Flag](https://app.codecov.io/gh/aio-libs/aiokafka/pull/960/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) | Coverage Δ | | |---|---|---| | [cext](https://app.codecov.io/gh/aio-libs/aiokafka/pull/960/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) | `91.71% <71.42%> (+0.08%)` | :arrow_up: | | [integration](https://app.codecov.io/gh/aio-libs/aiokafka/pull/960/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) | `94.68% <71.42%> (+0.10%)` | :arrow_up: | | [purepy](https://app.codecov.io/gh/aio-libs/aiokafka/pull/960/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) | `94.54% <71.42%> (+0.10%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/aio-libs/aiokafka/pull/960/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) | `44.85% <71.42%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vmaurin commented 5 months ago

@ods Is there some resource about performance ? We are using lz4 in production on our end, mainly for performance reason and I wonder how cramjam will perform compared to the current lz4 lib

vmaurin commented 5 months ago

Replying to myself https://github.com/milesgranger/cramjam/blob/master/cramjam-python/benchmarks/README.md#lz4

ods commented 5 months ago

I think looking to cramjam benchmarks is not enough here, as I also switched from level=0 to level=9, because 9 is used by broker by default. Level is not configurable yet, so this may also affect you.

vmaurin commented 5 months ago

@ods Noted. We might get or lose few percents it seems, but it won't be probably a x factor increase, so I guess we can just wait to see how well it works for our cases. For us it is mostly read/uncompress, where we rebuilt an internal streaming state from a kafka changelog topic when our worker starts