Thread limit introspection API, part 1: API scope#213
Conversation
There was a problem hiding this comment.
I find it a bit disturbing that calling threadpoolctl.threadpool_info can temporarily mutate the libraries global state in a non-thread safe way.
The alternative would be to hardcode expected scopes in LibController for all known libraries (and return "unknown" otherwise) and only call _determine_api_scope in tests or by passing a non-default kwarg value: threadpool_ctl.threadpool_info(api_scopes="effective") (and api_scopes="expected" would be the default) for instance.
| [ | ||
| ({"internal_api": "openblas"}, "process"), | ||
| ({"internal_api": "mkl"}, "process"), | ||
| ({"internal_api": "blis"}, "process"), |
There was a problem hiding this comment.
Shouldn't the value of expected_api_scope also depends on the current threading layer of each BLAS runtime?
There was a problem hiding this comment.
Yeah this was just an initial sketch to see what happens when it runs in CI.
| # Choose a desired number of threads that is different than the current | ||
| # number, and hopefully achievable under the current configuration: | ||
| if previous < 2: | ||
| expected = 2 |
There was a problem hiding this comment.
Can we safely assume that all runtimes allow for growing the number of threads even if externally limited, e.g. by some environment variables?
I tried with libomp and openblas + pthreads and it seems that I can use threadpoolctl.threadpool_limits(2) to grow the number of threads beyond what is set by OMP_NUM_THREADS=1 OPENBLAS_NUM_THREADS=1 but I wonder if this always the case.
| # thread pool implementation that is aware of cgroups, in which case a | ||
| # Docker container limited to one core will pass the safety check at | ||
| # the start of the function. Perhaps cpu_count() from loky should be | ||
| # moved into this package... |
There was a problem hiding this comment.
I agree this will be the best solution, but we can wait until the need actually arises.
|
|
||
| An attempt will be made to restore all settings to their previous state. | ||
|
|
||
| This won't work if you only have one core available. |
There was a problem hiding this comment.
Since this function can temporarily mutate global process state, it is not thread safe and can have a side effect on concurrently running code that use the libraries being dynamically inspected. I think we should document those limitations explicitly.
|
Another interesting finding by looking at the CI results: BLIS is |
|
I think it's useful to have it in |
2 participants
Fixes #211.
Add info to determine the scope of the thread-limit-setting API (thread local, process-wide, unknown). Introspecting the semantics of the thread pool are going to be a separate PR, assuming that can be done reasonably.