Skip to content
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

Add different dict modes to compression ratio regression test, update results.csv #2559

Merged
merged 3 commits into from
Mar 25, 2021
Merged

Add different dict modes to compression ratio regression test, update results.csv #2559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1cadf86
Add tests to regression tests for dict
senhuang42 Mar 25, 2021
f27e326
Restrict dictmode regression tests only to advanced API, fix some com…
senhuang42 Mar 25, 2021
bbbd578
Update results.csv
senhuang42 Mar 25, 2021
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
2 changes: 1 addition & 1 deletion programs/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ void UTIL_mirrorSourceFilesDirectories(const char** inFileNames, unsigned int nb
}

FileNamesTable*
UTIL_createExpandedFNT(const char** inputNames, size_t nbIfns, int followLinks)
UTIL_createExpandedFNT(const char* const* inputNames, size_t nbIfns, int followLinks)
{
unsigned nbFiles;
char* buf = (char*)malloc(LIST_SIZE_INCREASE);
Expand Down
2 changes: 1 addition & 1 deletion programs/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ void UTIL_refFilename(FileNamesTable* fnt, const char* filename);
* or NULL in case of error
*/
FileNamesTable*
UTIL_createExpandedFNT(const char** filenames, size_t nbFilenames, int followLinks);
UTIL_createExpandedFNT(const char* const* filenames, size_t nbFilenames, int followLinks);


/*-****************************************
Expand Down
78 changes: 63 additions & 15 deletions tests/regression/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,68 @@
};

/* Define a config for each level we want to test with. */
#define LEVEL(x) \
param_value_t const level_##x##_param_values[] = { \
{.param = ZSTD_c_compressionLevel, .value = x}, \
}; \
config_t const level_##x = { \
.name = "level " #x, \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values), \
}; \
config_t const level_##x##_dict = { \
.name = "level " #x " with dict", \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values), \
.use_dictionary = 1, \
#define LEVEL(x) \
param_value_t const level_##x##_param_values[] = { \
{.param = ZSTD_c_compressionLevel, .value = x}, \
}; \
param_value_t const level_##x##_param_values_dms[] = { \
{.param = ZSTD_c_compressionLevel, .value = x}, \
{.param = ZSTD_c_enableDedicatedDictSearch, .value = 0}, \
{.param = ZSTD_c_forceAttachDict, .value = ZSTD_dictForceAttach}, \
}; \
param_value_t const level_##x##_param_values_dds[] = { \
{.param = ZSTD_c_compressionLevel, .value = x}, \
{.param = ZSTD_c_enableDedicatedDictSearch, .value = 1}, \
{.param = ZSTD_c_forceAttachDict, .value = ZSTD_dictForceAttach}, \
}; \
param_value_t const level_##x##_param_values_dictcopy[] = { \
{.param = ZSTD_c_compressionLevel, .value = x}, \
{.param = ZSTD_c_enableDedicatedDictSearch, .value = 0}, \
{.param = ZSTD_c_forceAttachDict, .value = ZSTD_dictForceCopy}, \
}; \
param_value_t const level_##x##_param_values_dictload[] = { \
{.param = ZSTD_c_compressionLevel, .value = x}, \
{.param = ZSTD_c_enableDedicatedDictSearch, .value = 0}, \
{.param = ZSTD_c_forceAttachDict, .value = ZSTD_dictForceLoad}, \
}; \
config_t const level_##x = { \
.name = "level " #x, \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values), \
}; \
config_t const level_##x##_dict = { \
.name = "level " #x " with dict", \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values), \
.use_dictionary = 1, \
}; \
config_t const level_##x##_dict_dms = { \
.name = "level " #x " with dict dms", \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values_dms), \
.use_dictionary = 1, \
.advanced_api_only = 1, \
}; \
config_t const level_##x##_dict_dds = { \
.name = "level " #x " with dict dds", \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values_dds), \
.use_dictionary = 1, \
.advanced_api_only = 1, \
}; \
config_t const level_##x##_dict_copy = { \
.name = "level " #x " with dict copy", \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values_dictcopy), \
.use_dictionary = 1, \
.advanced_api_only = 1, \
}; \
config_t const level_##x##_dict_load = { \
.name = "level " #x " with dict load", \
.cli_args = "-" #x, \
.param_values = PARAM_VALUES(level_##x##_param_values_dictload), \
.use_dictionary = 1, \
.advanced_api_only = 1, \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I added the new configs to LEVEL() so we automatically get testing for all the levels we try.

An alternative approach is to have each of these just be their own config, with a fixed compression level (and if we wanted to test more clevels/strategies, we could just add more configs). But that does have the downside of not automatically hitting all the tested clevels.

I do prefer having these run on all the compression levels, in case in the future, strategies get changed in a way that might affect the dictionary strategies.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense

};

#define PARAM_VALUES(pv) \
Expand Down Expand Up @@ -194,7 +242,7 @@ static config_t explicit_params = {
static config_t const* g_configs[] = {

#define FAST_LEVEL(x) &level_fast##x, &level_fast##x##_dict,
#define LEVEL(x) &level_##x, &level_##x##_dict,
#define LEVEL(x) &level_##x, &level_##x##_dict, &level_##x##_dict_dms, &level_##x##_dict_dds, &level_##x##_dict_copy, &level_##x##_dict_load,
#include "levels.h"
#undef LEVEL
#undef FAST_LEVEL
Expand Down
5 changes: 5 additions & 0 deletions tests/regression/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ typedef struct {
* when the method allows it. Defaults to yes.
*/
int no_pledged_src_size;
/**
* Boolean parameter that says that this config should only be used
* for methods that use the advanced compression API
*/
int advanced_api_only;
} config_t;

/**
Expand Down
1 change: 1 addition & 0 deletions tests/regression/data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h> /* free() */

#include <sys/stat.h>

Expand Down
15 changes: 14 additions & 1 deletion tests/regression/method.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ static result_t simple_compress(method_state_t* base, config_t const* config) {
*/
if (base->data->type != data_type_file)
return result_error(result_error_skip);

if (config->advanced_api_only)
return result_error(result_error_skip);

if (config->use_dictionary || config->no_pledged_src_size)
return result_error(result_error_skip);
Expand Down Expand Up @@ -151,6 +154,9 @@ static result_t compress_cctx_compress(

if (base->data->type != data_type_dir)
return result_error(result_error_skip);

if (config->advanced_api_only)
return result_error(result_error_skip);

int const level = config_get_level(config);

Expand Down Expand Up @@ -254,6 +260,9 @@ static result_t cli_compress(method_state_t* state, config_t const* config) {
if (config->cli_args == NULL)
return result_error(result_error_skip);

if (config->advanced_api_only)
return result_error(result_error_skip);

/* We don't support no pledged source size with directories. Too slow. */
if (state->data->type == data_type_dir && config->no_pledged_src_size)
return result_error(result_error_skip);
Expand Down Expand Up @@ -523,6 +532,10 @@ static result_t old_streaming_compress_internal(
result = result_error(result_error_skip);
goto out;
}
if (config->advanced_api_only) {
result = result_error(result_error_skip);
goto out;
}
if (init_cstream(state, zcs, config, advanced, cdict ? &cd : NULL)) {
result = result_error(result_error_compression_error);
goto out;
Expand Down Expand Up @@ -651,7 +664,7 @@ method_t const old_streaming_advanced = {
};

method_t const old_streaming_cdict = {
.name = "old streaming cdcit",
.name = "old streaming cdict",
.create = buffer_state_create,
.compress = old_streaming_compress_cdict,
.destroy = buffer_state_destroy,
Expand Down
Loading