szcompressor / SZ

Error-bounded Lossy Data Compressor (for floating-point/integer datasets)
http://szcompressor.org
Other
155 stars 56 forks source link

API is not thread-safe #29

Closed martinradev closed 3 years ago

martinradev commented 5 years ago

The compress and decompress functions are not thread-safe. The reason for this is that a global context confparams_cpr, confparams_dec, exe_params is modified via some of the calls. Example:

unsigned char* SZ_compress_args(int dataType, void *data, size_t *outSize, int errBoundMode, double absErrBound, 
double relBoundRatio, double pwrBoundRatio, size_t r5, size_t r4, size_t r3, size_t r2, size_t r1)
{
    confparams_cpr->dataType = dataType;
void *SZ_decompress(int dataType, unsigned char *bytes, size_t byteLength, size_t r5, size_t r4, size_t r3, size_t r2, size_t r1)
{
    if(confparams_dec==NULL)
        confparams_dec = (sz_params*)malloc(sizeof(sz_params));
    memset(confparams_dec, 0, sizeof(sz_params));
    if(exe_params==NULL)
        exe_params = (sz_exedata*)malloc(sizeof(sz_exedata));
    memset(exe_params, 0, sizeof(sz_exedata));
    exe_params->SZ_SIZE_TYPE = 8;

I think it would be beneficial to guarantee that calling the compress/decompress is thread safe. Otherwise, the user would have to introduce a single lock for the compress / decompress paths which would lead to contention and kill multi-threaded performance.

martinradev commented 5 years ago

A solution would be to modify the API. Instead of keeping a global context stored in the library, the API could be extended so that ContextInit returns a context handle to the user. The context handle is to be passed to the compress/decompress functions. The handle can additionally be bound to a mutex so that within the implementation of the compress/decompress functions, the mutex is acquired. This achieves two things:

disheng222 commented 3 years ago

Hi, We fixed the thread-safe issue recently. Please check the github master branch. There are multiple error bound modes. Specifically, ABS (absolute error bound) and REL (value-range based error bound) is supposed to be thread safe now. fix-PSNR mode is still not thread safe. We added two example codes test_threadsafe.c and test_threadsafe2.c in the example/ directory.

disheng222 commented 3 years ago

Forgot to mention that the thread safe support is mainly provided for single-precision (float) and double-precision (double) in the current implementation.

mrmbernardi commented 1 year ago

Is the PSNR mode currently thread safe?