Skip to content

zstd support #539

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 27, 2023
Merged

zstd support #539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
84a73c0
zstd support
rlerdorf Apr 25, 2023
d78dff7
add bit for compression_level option
rlerdorf Apr 25, 2023
91dfd11
Always define the ZSTD const
rlerdorf Apr 26, 2023
0869f85
Here too, always define it
rlerdorf Apr 26, 2023
72da554
Error if we see zstd-compressed values but don't have zstd support
rlerdorf Apr 26, 2023
c3c285f
fix ws
rlerdorf Apr 26, 2023
a8a0d12
Fix build
rlerdorf Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ PHP_ARG_ENABLE(memcached-protocol, whether to enable memcached protocol support,
PHP_ARG_WITH(system-fastlz, whether to use system FastLZ library,
[ --with-system-fastlz Use system FastLZ library], no, no)

PHP_ARG_WITH(zstd, whether to use system zstd library,
[ --with-zstd Use system zstd library], no, no)

if test -z "$PHP_ZLIB_DIR"; then
PHP_ARG_WITH(zlib-dir, for ZLIB,
[ --with-zlib-dir=DIR Set the path to ZLIB install prefix.], no)
Expand Down Expand Up @@ -345,6 +348,13 @@ if test "$PHP_MEMCACHED" != "no"; then
PHP_MEMCACHED_FILES="${PHP_MEMCACHED_FILES} fastlz/fastlz.c"
fi

if test "$PHP_ZSTD" != "no"; then
AC_CHECK_HEADERS([zstd.h], [ac_cv_have_zstd="yes"], [ac_cv_have_zstd="no"])
PHP_CHECK_LIBRARY(zstd, ZSTD_compress,
[PHP_ADD_LIBRARY(zstd, 1, MEMCACHED_SHARED_LIBADD)],
[AC_MSG_ERROR(zstd library not found)])
fi

if test "$PHP_MEMCACHED_SESSION" != "no"; then
PHP_MEMCACHED_FILES="${PHP_MEMCACHED_FILES} php_memcached_session.c"
fi
Expand Down
78 changes: 76 additions & 2 deletions php_memcached.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
#endif
#include <zlib.h>

#ifdef HAVE_ZSTD_H
#include <zstd.h>
#endif

#ifdef HAVE_JSON_API
# include "ext/json/php_json.h"
#endif
Expand Down Expand Up @@ -77,6 +81,7 @@ static int php_memc_list_entry(void) {
#define MEMC_OPT_COMPRESSION_TYPE -1004
#define MEMC_OPT_STORE_RETRY_COUNT -1005
#define MEMC_OPT_USER_FLAGS -1006
#define MEMC_OPT_COMPRESSION_LEVEL -1007

/****************************************
Custom result codes
Expand Down Expand Up @@ -107,6 +112,7 @@ static int php_memc_list_entry(void) {
#define MEMC_VAL_COMPRESSED (1<<0)
#define MEMC_VAL_COMPRESSION_ZLIB (1<<1)
#define MEMC_VAL_COMPRESSION_FASTLZ (1<<2)
#define MEMC_VAL_COMPRESSION_ZSTD (1<<3)

#define MEMC_VAL_GET_FLAGS(internal_flags) (((internal_flags) & MEMC_MASK_INTERNAL) >> 4)
#define MEMC_VAL_SET_FLAG(internal_flags, internal_flag) ((internal_flags) |= (((internal_flag) << 4) & MEMC_MASK_INTERNAL))
Expand Down Expand Up @@ -152,6 +158,7 @@ typedef struct {

zend_long serializer;
zend_long compression_type;
zend_long compression_level;

zend_long store_retry_count;
zend_long set_udf_flags;
Expand Down Expand Up @@ -278,6 +285,10 @@ static PHP_INI_MH(OnUpdateCompressionType)
MEMC_G(compression_type) = COMPRESSION_TYPE_FASTLZ;
} else if (!strcmp(ZSTR_VAL(new_value), "zlib")) {
MEMC_G(compression_type) = COMPRESSION_TYPE_ZLIB;
#ifdef HAVE_ZSTD_H
} else if (!strcmp(ZSTR_VAL(new_value), "zstd")) {
MEMC_G(compression_type) = COMPRESSION_TYPE_ZSTD;
#endif
} else {
return FAILURE;
}
Expand Down Expand Up @@ -408,6 +419,7 @@ PHP_INI_BEGIN()

MEMC_INI_ENTRY("compression_type", "fastlz", OnUpdateCompressionType, compression_name)
MEMC_INI_ENTRY("compression_factor", "1.3", OnUpdateReal, compression_factor)
MEMC_INI_ENTRY("compression_level", "3", OnUpdateLong, compression_level)
MEMC_INI_ENTRY("compression_threshold", "2000", OnUpdateLong, compression_threshold)
MEMC_INI_ENTRY("serializer", SERIALIZER_DEFAULT_NAME, OnUpdateSerializer, serializer_name)
MEMC_INI_ENTRY("store_retry_count", "0", OnUpdateLong, store_retry_count)
Expand Down Expand Up @@ -897,6 +909,19 @@ zend_bool s_compress_value (php_memc_compression_type compression_type, zend_str
}
break;

#ifdef HAVE_ZSTD_H
case COMPRESSION_TYPE_ZSTD:
{
compressed_size = ZSTD_compress((void *)buffer, buffer_size, ZSTR_VAL(payload), ZSTR_LEN(payload), MEMC_G(compression_level));

if (!ZSTD_isError(compressed_size)) {
compress_status = 1;
compression_type_flag = MEMC_VAL_COMPRESSION_ZSTD;
}
}
break;
#endif

case COMPRESSION_TYPE_ZLIB:
{
compressed_size = buffer_size;
Expand Down Expand Up @@ -2939,6 +2964,9 @@ static PHP_METHOD(Memcached, getOption)
case MEMC_OPT_COMPRESSION_TYPE:
RETURN_LONG(memc_user_data->compression_type);

case MEMC_OPT_COMPRESSION_LEVEL:
RETURN_LONG(memc_user_data->compression_level);

case MEMC_OPT_COMPRESSION:
RETURN_BOOL(memc_user_data->compression_enabled);

Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

lval == COMPRESSION_TYPE_ZLIB) {
memc_user_data->compression_type = lval;
} else {
Expand Down Expand Up @@ -3608,16 +3639,24 @@ zend_string *s_decompress_value (const char *payload, size_t payload_len, uint32
uint32_t stored_length;
unsigned long length;
zend_bool decompress_status = 0;
zend_bool is_fastlz = 0, is_zlib = 0;
zend_bool is_fastlz = 0, is_zlib = 0, is_zstd = 0;

if (payload_len < sizeof (uint32_t)) {
return NULL;
}

is_fastlz = MEMC_VAL_HAS_FLAG(flags, MEMC_VAL_COMPRESSION_FASTLZ);
is_zstd = MEMC_VAL_HAS_FLAG(flags, MEMC_VAL_COMPRESSION_ZSTD);
is_zlib = MEMC_VAL_HAS_FLAG(flags, MEMC_VAL_COMPRESSION_ZLIB);

if (!is_fastlz && !is_zlib) {
#ifndef HAVE_ZSTD_H
if (is_zstd) {
php_error_docref(NULL, E_WARNING, "could not decompress value: value was compressed with zstd but zstd support has not been compiled in");
return NULL;
}
#endif

if (!is_fastlz && !is_zlib && !is_zstd) {
php_error_docref(NULL, E_WARNING, "could not decompress value: unrecognised compression type");
return NULL;
}
Expand All @@ -3629,6 +3668,23 @@ zend_string *s_decompress_value (const char *payload, size_t payload_len, uint32

buffer = zend_string_alloc (stored_length, 0);

#ifdef HAVE_ZSTD_H
if (is_zstd) {
length = ZSTD_getFrameContentSize(payload, payload_len);
if (length == ZSTD_CONTENTSIZE_ERROR) {
php_error_docref(NULL, E_WARNING, "value was not compressed by zstd");
zend_string_release (buffer);
return NULL;
} else if (length == ZSTD_CONTENTSIZE_UNKNOWN) {
php_error_docref(NULL, E_WARNING, "zstd streaming decompression not supported");
zend_string_release (buffer);
return NULL;
}
decompress_status = !ZSTD_isError(ZSTD_decompress(&buffer->val, buffer->len, payload, payload_len));

}
else
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (is_fastlz) {
decompress_status = ((length = fastlz_decompress(payload, payload_len, &buffer->val, buffer->len)) > 0);
}
Expand Down Expand Up @@ -3955,6 +4011,7 @@ PHP_GINIT_FUNCTION(php_memcached)
php_memcached_globals->memc.compression_threshold = 2000;
php_memcached_globals->memc.compression_type = COMPRESSION_TYPE_FASTLZ;
php_memcached_globals->memc.compression_factor = 1.30;
php_memcached_globals->memc.compression_level = 3;
php_memcached_globals->memc.store_retry_count = 2;

php_memcached_globals->memc.sasl_initialised = 0;
Expand Down Expand Up @@ -4000,6 +4057,7 @@ static void php_memc_register_constants(INIT_FUNC_ARGS)

REGISTER_MEMC_CLASS_CONST_LONG(OPT_COMPRESSION, MEMC_OPT_COMPRESSION);
REGISTER_MEMC_CLASS_CONST_LONG(OPT_COMPRESSION_TYPE, MEMC_OPT_COMPRESSION_TYPE);
REGISTER_MEMC_CLASS_CONST_LONG(OPT_COMPRESSION_LEVEL, MEMC_OPT_COMPRESSION_LEVEL);
REGISTER_MEMC_CLASS_CONST_LONG(OPT_PREFIX_KEY, MEMC_OPT_PREFIX_KEY);
REGISTER_MEMC_CLASS_CONST_LONG(OPT_SERIALIZER, MEMC_OPT_SERIALIZER);

Expand All @@ -4015,6 +4073,15 @@ static void php_memc_register_constants(INIT_FUNC_ARGS)
REGISTER_MEMC_CLASS_CONST_BOOL(HAVE_IGBINARY, 0);
#endif

/*
* Indicate whether zstd compression is available
*/
#ifdef HAVE_ZSTD_H
REGISTER_MEMC_CLASS_CONST_BOOL(HAVE_ZSTD, 1);
#else
REGISTER_MEMC_CLASS_CONST_BOOL(HAVE_ZSTD, 0);
#endif

/*
* Indicate whether json serializer is available
*/
Expand Down Expand Up @@ -4186,6 +4253,7 @@ static void php_memc_register_constants(INIT_FUNC_ARGS)
*/
REGISTER_MEMC_CLASS_CONST_LONG(COMPRESSION_FASTLZ, COMPRESSION_TYPE_FASTLZ);
REGISTER_MEMC_CLASS_CONST_LONG(COMPRESSION_ZLIB, COMPRESSION_TYPE_ZLIB);
REGISTER_MEMC_CLASS_CONST_LONG(COMPRESSION_ZSTD, COMPRESSION_TYPE_ZSTD);

/*
* Flags.
Expand Down Expand Up @@ -4351,6 +4419,12 @@ PHP_MINFO_FUNCTION(memcached)
php_info_print_table_row(2, "msgpack support", "no");
#endif

#ifdef HAVE_ZSTD_H
php_info_print_table_row(2, "zstd support", "yes");
#else
php_info_print_table_row(2, "zstd support", "no");
#endif

php_info_print_table_end();

DISPLAY_INI_ENTRIES();
Expand Down
4 changes: 3 additions & 1 deletion php_memcached_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ typedef enum {

typedef enum {
COMPRESSION_TYPE_ZLIB = 1,
COMPRESSION_TYPE_FASTLZ = 2
COMPRESSION_TYPE_FASTLZ = 2,
COMPRESSION_TYPE_ZSTD = 3
} php_memc_compression_type;

typedef struct {
Expand Down Expand Up @@ -186,6 +187,7 @@ ZEND_BEGIN_MODULE_GLOBALS(php_memcached)
zend_long compression_threshold;
double compression_factor;
zend_long store_retry_count;
zend_long compression_level;

/* Converted values*/
php_memc_serializer_type serializer_type;
Expand Down
2 changes: 2 additions & 0 deletions tests/compression_conditions.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ function get_compression($name) {
return Memcached::COMPRESSION_ZLIB;
case 'fastlz':
return Memcached::COMPRESSION_FASTLZ;
case 'zstd':
return Memcached::COMPRESSION_ZSTD;
default:
echo "Strange compression type: $name\n";
return 0;
Expand Down
34 changes: 34 additions & 0 deletions tests/compression_types.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ function get_compression($name) {
return Memcached::COMPRESSION_ZLIB;
case 'fastlz':
return Memcached::COMPRESSION_FASTLZ;
case 'zstd':
if (Memcached::HAVE_ZSTD) {
return Memcached::COMPRESSION_ZSTD;
} else return 0;
default:
echo "Strange compression type: $name\n";
return 0;
Expand Down Expand Up @@ -54,6 +58,26 @@ fetch_with_compression($m, 'hello6', $data, '', 'fastlz');
fetch_with_compression($m, 'hello7', $data, 'zlib', '');
fetch_with_compression($m, 'hello8', $data, 'fastlz', '');
fetch_with_compression($m, 'hello9', $data, '', '');
if (Memcached::HAVE_ZSTD) {
fetch_with_compression($m, 'hello10', $data, 'zstd', 'zstd');
fetch_with_compression($m, 'hello11', $data, 'zstd', 'fastlz');
fetch_with_compression($m, 'hello12', $data, 'fastlz', 'zstd');
fetch_with_compression($m, 'hello13', $data, '', 'zstd');
fetch_with_compression($m, 'hello14', $data, 'zstd', '');
} else {
echo <<<EOB
set=[zstd] get=[zstd]
bool(true)
set=[zstd] get=[fastlz]
bool(true)
set=[fastlz] get=[zstd]
bool(true)
set=[] get=[zstd]
bool(true)
set=[zstd] get=[]
bool(true)
EOB;
}
?>
--EXPECT--
set=[zlib] get=[zlib]
Expand All @@ -74,3 +98,13 @@ set=[fastlz] get=[]
bool(true)
set=[] get=[]
bool(true)
set=[zstd] get=[zstd]
bool(true)
set=[zstd] get=[fastlz]
bool(true)
set=[fastlz] get=[zstd]
bool(true)
set=[] get=[zstd]
bool(true)
set=[zstd] get=[]
bool(true)