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.

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
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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
77 changes: 75 additions & 2 deletions php_memcached.c
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,9 @@ 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)
#ifdef HAVE_ZSTD_H
#define MEMC_VAL_COMPRESSION_ZSTD (1<<3)
#endif

#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 +160,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 +287,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 +421,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 +911,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 +2966,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 +3031,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 +3641,19 @@ 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);
#ifdef HAVE_ZSTD_H
is_zstd = MEMC_VAL_HAS_FLAG(flags, MEMC_VAL_COMPRESSION_ZSTD);
#endif
is_zlib = MEMC_VAL_HAS_FLAG(flags, MEMC_VAL_COMPRESSION_ZLIB);

if (!is_fastlz && !is_zlib) {
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 +3665,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 +4008,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 +4054,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 +4070,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 +4250,9 @@ 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);
#ifdef HAVE_ZSTD_H
REGISTER_MEMC_CLASS_CONST_LONG(COMPRESSION_ZSTD, COMPRESSION_TYPE_ZSTD);
#endif

/*
* Flags.
Expand Down Expand Up @@ -4351,6 +4418,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
6 changes: 5 additions & 1 deletion php_memcached_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ typedef enum {

typedef enum {
COMPRESSION_TYPE_ZLIB = 1,
COMPRESSION_TYPE_FASTLZ = 2
COMPRESSION_TYPE_FASTLZ = 2,
#ifdef HAVE_ZSTD_H
COMPRESSION_TYPE_ZSTD = 3
#endif
} php_memc_compression_type;

typedef struct {
Expand Down Expand Up @@ -186,6 +189,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
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
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)