ENH add initializer limiting n_threads for C-libs by tomMoral · Pull Request #701 · joblib/joblib

tomMoral · 2018-06-20T14:09:01Z

This functionnality supports OpenMP, OpenBLAS, MKL and Accelerated
Set n_threads=1 by default in all workers to avoid oversubscrption

codecov · 2018-06-20T14:22:21Z

Codecov Report

Merging #701 into master will decrease coverage by 0.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
- Coverage   95.14%   95.02%   -0.13%     
==========================================
  Files          40       40              
  Lines        5725     5746      +21     
==========================================
+ Hits         5447     5460      +13     
- Misses        278      286       +8

Impacted Files	Coverage Δ
joblib/test/test_parallel.py	`96.62% <100%> (+0.07%)`	⬆️
joblib/executor.py	`100% <100%> (ø)`	⬆️
joblib/_parallel_backends.py	`94.8% <100%> (-2.32%)`	⬇️
joblib/backports.py	`93.75% <0%> (-2.09%)`	⬇️
joblib/_store_backends.py	`90.47% <0%> (-0.53%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3eea7c...5321aad. Read the comment docs.

ogrisel · 2018-06-20T14:58:02Z

joblib/_parallel_backends.py

        if self._pool is None:
-            self._pool = ThreadPool(self._n_jobs)
+            self._pool = ThreadPool(
+                self._n_jobs, initializer=self.limit_clib_threads)


I don't think threads have isolated environment variables. Dynamic / ctypes based calls would be required to limit clib threads while joblib is doing thread-based parallelism.

I would just not register any initializer at the ThreadPool level for now.

I confirm that there is a single environment namespace per-process: all threads of the same process share the same environment variables:

>>> from time import sleep >>> import os >>> from threading import get_ident >>> from multiprocessing.pool import ThreadPool >>> def env_tweaks(*args): ... tid = str(get_ident()) ... os.environ['THREAD_ID'] = tid ... sleep(0.01) ... return (tid, os.environ['THREAD_ID']) ... ... >>> p = ThreadPool(4) >>> results = p.map(env_tweaks, range(10)) >>> results [('140286315460352', '140286290282240'), ('140286307067648', '140286315460352'), ('140286298674944', '140286290282240'), ('140286290282240', '140286307067648'), ('140286315460352', '140286298674944'), ('140286307067648', '140286315460352'), ('140286290282240', '140286307067648'), ('140286298674944', '140286307067648'), ('140286315460352', '140286307067648'), ('140286307067648', '140286307067648')] >>>

Indeed.. thanks for pointing that out!

ogrisel · 2018-06-20T16:13:48Z

Please add an entry to the changelog.

Please also add a new section at the end of the parallel.rst documentation to explain the concept of over-subscription of CPU resources by an excess number of concurrent processes and threads in general and the expected behavior of this simplistic mitigation strategy when joblib calls into third-party libraries that manage their own internal thread pools.

tomMoral · 2018-06-20T16:36:56Z

Yes I am planning on adding in this PR:

an entry in the changelog
an entry in the doc
an entry in the travis matrix to test this with cython and openmp.

- this functionnality supports OpenMP, OpenBLAS, MKL and Accelerated - Set n_threads=1 by default in all workers to avoid oversubscrption

ogrisel

LGTM besides the following the comments:

ogrisel · 2018-06-22T09:23:29Z

CHANGES.rst

+Thomas Moreau
+
+    Limit the number of threads used in worker processes (for ``loky`` and
+    ``multiprocessing``) by C-libraries that relies on threadpools. This


I would not mention multiprocessing as it's not reliable when using the default fork context under posix.

ogrisel · 2018-06-22T09:29:07Z

CHANGES.rst

+    Implement the ``loky`` backend with @ogrisel. This backend relies on
+    a robust implementation of ``concurrent.futures.ProcessPoolExecutor``
+    with spawned processes that can be reused accross the ``Parallel``
+    calls.


Please move this to the top and explain that it fixes bad interactions (crash) with the internal thread-pool of third-party libraries with a link to.

https://pythonhosted.org/joblib/parallel.html#bad-interaction-of-multiprocessing-and-third-party-libraries

ogrisel · 2018-06-22T09:30:42Z

doc/parallel.rst

+computational power available for each process. Moreover, when many processes
+are running, the time taken by the OS scheduler to switch between them can
+further hinder the performance of the computation. It is generally better to
+avoid using more ``Process/Thread`` than the number of CPU on a machine.


avoid using significantly more processes or threads than the number of CPUs on a machine.

ogrisel · 2018-06-22T09:31:17Z

doc/parallel.rst

+further hinder the performance of the computation. It is generally better to
+avoid using more ``Process/Thread`` than the number of CPU on a machine.
+
+Some third-partiy libraries -- *e.g.* ``numpy`` -- manage internally a


e.g. the BLAS runtime used by numpy

ogrisel · 2018-06-22T09:31:50Z

doc/parallel.rst

+avoid using more ``Process/Thread`` than the number of CPU on a machine.
+
+Some third-partiy libraries -- *e.g.* ``numpy`` -- manage internally a
+thread-pool to perform their computations. The default behavior is generaly to


* tag '0.12': (116 commits) Release 0.12 typo typo typo ENH add initializer limiting n_threads for C-libs (joblib#701) DOC better parallel docstring (joblib#704) [MRG] Nested parallel call thread bomb mitigation (joblib#700) MTN vendor loky2.1.3 (joblib#699) Make it possible to configure the reusable executor workers timeout (joblib#698) MAINT increase timeouts to make test more robust on travis DOC: use the .joblib extension instead of .pkl (joblib#697) [MRG] Fix exception handling in nested parallel calls (joblib#696) Fix skip test lz4 not installed (joblib#695) [MRG] numpy_pickle: several enhancements (joblib#626) Introduce Parallel.__call__ backend callbacks (joblib#689) Add distributed on readthedocs (joblib#686) Support registration of external backends (joblib#655) [MRG] Add a dask.distributed example (joblib#613) ENH use cloudpickle to pickle interactively defined callable (joblib#677) CI freeze the version of sklearn0.19.1 and scipy1.0.1 (joblib#685) ...

* releases: (121 commits) Release 0.12.1 fix kwonlydefaults key error in filter_args (joblib#715) MNT fix some "undefined name" flake8 warnings (joblib#713). from importlib import reload for Python 3 (joblib#675) MTN vendor loky2.1.4 (joblib#708) Release 0.12 typo typo typo ENH add initializer limiting n_threads for C-libs (joblib#701) DOC better parallel docstring (joblib#704) [MRG] Nested parallel call thread bomb mitigation (joblib#700) MTN vendor loky2.1.3 (joblib#699) Make it possible to configure the reusable executor workers timeout (joblib#698) MAINT increase timeouts to make test more robust on travis DOC: use the .joblib extension instead of .pkl (joblib#697) [MRG] Fix exception handling in nested parallel calls (joblib#696) Fix skip test lz4 not installed (joblib#695) [MRG] numpy_pickle: several enhancements (joblib#626) Introduce Parallel.__call__ backend callbacks (joblib#689) ...

ogrisel reviewed Jun 20, 2018

View reviewed changes

tomMoral added 2 commits June 21, 2018 14:56

ENH add initializer limiting n_threads for C-libs

71ee502

- this functionnality supports OpenMP, OpenBLAS, MKL and Accelerated - Set n_threads=1 by default in all workers to avoid oversubscrption

FIX remove ThreadPool init as it has no effect yet

45dc113

tomMoral force-pushed the PR_limit_threads_omp branch from e24abb5 to e7c933b Compare June 21, 2018 12:58

tomMoral added 8 commits June 21, 2018 14:58

TST test the thread limit in workers

e7c933b

DOC add entries in CHANGE.rst

5df3df7

FIX travis install.sh script

32159e7

FIX test parallel for omp

e65a91c

TST more information checked in thread limit

055c9ee

FIX no test on mp as it relies on fork and should not work

469daba

DOC add over-sub section in doc

dd53fda

CI increase timeout for pytest

b396357

ogrisel approved these changes Jun 22, 2018

View reviewed changes

DOC improve spelling and typo

5321aad

tomMoral merged commit 3cfb821 into joblib:master Jun 22, 2018

tomMoral deleted the PR_limit_threads_omp branch June 22, 2018 10:01

ogrisel mentioned this pull request May 29, 2019

Oversubscription protection done by setting environment variables in worker initializer is not effective #880

Closed

Conversation

tomMoral commented Jun 20, 2018

Uh oh!

codecov bot commented Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel commented Jun 20, 2018

Uh oh!

tomMoral commented Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jun 20, 2018 •

edited

Loading

tomMoral commented Jun 20, 2018 •

edited

Loading