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.

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

Enable ZSTD_MULTITHREAD by default? #2364

Closed
ghost opened this issue Oct 18, 2020 · 7 comments
Closed

Enable ZSTD_MULTITHREAD by default? #2364

ghost opened this issue Oct 18, 2020 · 7 comments
Labels

Comments

@ghost
Copy link

ghost commented Oct 18, 2020

On macOS/Ubuntu, when dynamically linking to the zstd library provided by system, multi-threaded compression can't be used:

    case ZSTD_c_nbWorkers :
#ifndef ZSTD_MULTITHREAD
        RETURN_ERROR_IF(value!=0, parameter_unsupported, "not compiled with multithreading");
        return 0;
#else
        FORWARD_IF_ERROR(ZSTD_cParam_clampBounds(param, &value), "");
        CCtxParams->nbWorkers = value;
        return CCtxParams->nbWorkers;
#endif

May I ask, what is the reason for not enabling it by default?

@Cyan4973
Copy link
Contributor

Multi-threading is not enabled by default in the library,
note though that it can be selected by defining ZSTD_MULTITHREAD during compilation.

There are many reasons to not enable multithreading by default, but good news is that we have been resolutely tackling them one by one over the past versions. So much so that we are now pretty close to being able to update this policy.

Such change will also require a larger version number change, like v1.5.0. So I would waive that it will probably happen sometimes in 2021.

@ghost
Copy link
Author

ghost commented Oct 18, 2020

Thanks, I see.

Python's built-in modules use system's libraries on Linux, such as sqlite3/liblzma. I have written a zstd module for Python standard library, it has this problem.

FYI, a recent discussion about adding zstd to Python standard library:
https://mail.python.org/archives/list/[email protected]/thread/VQIFA7WTNRAOYZGTVP4WZC2CD36KYIVY/

Please understand that people on that mailing list tend to be conservative and cautious, they are used to rejecting things, but are very open to new ideas.

@v-fox
Copy link

v-fox commented Oct 18, 2020

There are many reasons to not enable multithreading by default, but good news is that we have been resolutely tackling them one by one over the past versions. So much so that we are now pretty close to being able to update this policy.

Such change will also require a larger version number change, like v1.5.0. So I would waive that it will probably happen sometimes in 2021.

You mean just compile-in the option by default or '-T0' also being the default ? Because SO/HO users would prefer the latter, as expressed in #1423 Even if it's a compile-time hardcode instead of an envvar.

@ghost
Copy link
Author

ghost commented Dec 21, 2020

You mean just compile-in the option by default or '-T0' also being the default ? Because SO/HO users would prefer the latter, as expressed in #1423 Even if it's a compile-time hardcode instead of an envvar.

@v-fox.

It's not about CLI program.

Some developer users build zstd library, but multi-threaded compression is not enabled by default, then compiled program can only use single thread compression.

@Satyabhama-Reddy
Copy link

Satyabhama-Reddy commented Mar 31, 2021

There are many reasons to not enable multithreading by default, but good news is that we have been resolutely tackling them one by one over the past versions. So much so that we are now pretty close to being able to update this policy.

@Cyan4973 ,

Could you explain more on the reasons? Would there be any impact of enabling it during build and not using the functionality?
(I am currently using v1.4.9)

@Cyan4973
Copy link
Contributor

Cyan4973 commented Mar 31, 2021

There used to be an impact, but it's fixed now.

So indeed, nowadays, libzstd v1.4.9 can be compiled with multi-threading enabled, and existing applications can still link to the resulting library normally, as they use to with the "default" non-multithreaded version. These unmodified applications will trigger the same execution path, using single-thread by default. Multi-thread execution is only invoked when explicitly requested.

@ghost
Copy link
Author

ghost commented May 12, 2021

Solved in #2584

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants