zstd support #539
zstd support #539
Conversation
|
|
||
| } | ||
| else | ||
| #endif |
There was a problem hiding this comment.
(for existing block below) -- I am refreshing myself on the other compression routines, but it looks like they are hard requirements to compile the php-memcached package and so they're never conditional on the libraries. Is that because PHP always offers these routines? (I'll catch up on reading, asking out loud for transparency)
There was a problem hiding this comment.
Yes, both zlib and fastlz are mandatory. If the system fastlz is not available it will use a bundled copy. In theory we could do the same with zstd, but it is pretty widely available in every distro these days, so we could also just make it a hard requirement.
| @@ -3001,6 +3029,9 @@ int php_memc_set_option(php_memc_object_t *intern, long option, zval *value) | |||
| case MEMC_OPT_COMPRESSION_TYPE: | |||
| lval = zval_get_long(value); | |||
| if (lval == COMPRESSION_TYPE_FASTLZ || | |||
| #ifdef HAVE_ZSTD_H | |||
| lval == COMPRESSION_TYPE_ZSTD || | |||
| #endif | |||
There was a problem hiding this comment.
Looks like the scenario here is setting the compression type to zstd, but it's not compiled in, you get an invalid argument error. Seems reasonable!
There was a problem hiding this comment.
lgtm! let me know if you have any other work you want to do on the PR, otherwise I'll go ahead and land it
I am not planning anything else for the moment. There is some interesting stuff that could be done around training zstd and using dictionary compression, but that is probably overkill here.
…540) Make it possible to use setOption to set Memcached::OPT_COMPRESSION_LEVEL which was missed in the original zstd PR #539 zlib compression was using the default zlib compression level of 6. With this PR it is now possible to choose other levels for zlib as well. The default remains at 6 so nothing will change for people upgrading unless they explicitly set a different level. Here is some more benchmarking data using php serialized data https://gist.github.com/rlerdorf/b9bae385446d5a30b65e6e241e34d0a8 fastlz is not really useful at any value size anymore. Anybody looking for lightning quick compression and decompression should use zstd at level 1. compression_level is not applied to fastlz because it only has 2 levels and php-memcached already switches from level 1 to 2 automatically for values larger than 65535 bytes. Forcing it to one or the other doesn't seem useful.
- Add #515 option to locally enforce payload size limit - Add #539 zstd support - Add #540 compression_level option - Mark password as a sensitive param for PHP 8.2 - Fix Windows PHP 8 compatibility - Fix #518 Windows msgpack support - Fix #522 signed integer overflow - Fix #523 incorrect PHP reflection type for Memcached::cas $cas_token - Fix #546 don't check key automatically, unless client-side verify_key is enabled - Fix #555 incompatible pointer types (32-bit)
- Add #515 option to locally enforce payload size limit - Add #539 zstd support - Add #540 compression_level option - Mark password as a sensitive param for PHP 8.2 - Fix Windows PHP 8 compatibility - Fix #518 Windows msgpack support - Fix #522 signed integer overflow - Fix #523 incorrect PHP reflection type for Memcached::cas $cas_token - Fix #546 don't check key automatically, unless client-side verify_key is enabled
This adds zstd compression support.
The current two options, zlib and fastlz is basically a choice between performance and compression ratio. You would choose zlib if you are memory-bound and fastlz if you are cpu-bound.
With zstd, you get the performance of fastlz with the compression of zlib. And often it wins on both. See this benchmark I ran on json files of varying sizes:
https://gist.github.com/rlerdorf/788f3d0144f9c5514d8fee9477cbe787
Taking just a 40k json blob, we see that zstd at compression level 3 reduces it to 8862 bytes. Our current zlib 1 gets worse compression at 10091 bytes and takes longer both to compress and decompress.
The fact that decompression a dramatically faster with zstd is a win for most common memcache uses since they tend to be read-heavy.
The PR also adds a
memcache.compression_levelINI switch which currently only applies to zstd compression. It could probably be made to also apply to zlib and fastlz.