Skip to content

Conversation

ogrisel
Copy link
Contributor

@ogrisel ogrisel commented Jun 20, 2018

This does not protect against over-subscription but at least it protects against recursive thread bombs as reported initially in the tests of #690.

@ogrisel
Copy link
Contributor Author

ogrisel commented Jun 20, 2018

@aabadie @lesteve circleci crashed for some reason that seems unrelated to this PR by I don't have the rights to trigger a rebuild with my github account.

@codecov
Copy link

codecov bot commented Jun 20, 2018

Codecov Report

Merging #700 into master will increase coverage by 0.27%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #700      +/-   ##
=========================================
+ Coverage   95.02%   95.3%   +0.27%     
=========================================
  Files          40      40              
  Lines        5694    5725      +31     
=========================================
+ Hits         5411    5456      +45     
+ Misses        283     269      -14
Impacted Files Coverage Δ
joblib/test/test_parallel.py 96.55% <100%> (+0.13%) ⬆️
joblib/_parallel_backends.py 98.35% <100%> (+2.59%) ⬆️
joblib/test/test_memory.py 98.16% <0%> (+0.36%) ⬆️
joblib/_store_backends.py 91% <0%> (+0.52%) ⬆️
joblib/backports.py 95.83% <0%> (+2.08%) ⬆️
joblib/disk.py 88.33% <0%> (+6.66%) ⬆️

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 fb0c50e...6ce5c98. Read the comment docs.

Copy link
Contributor

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

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

LGTM. I like this solution as it avoids adding complexity in the code.

"""A horrible function that does recursive parallel calls"""
return Parallel()(delayed(_recursive_parallel)() for i in range(2))


Copy link
Contributor

Choose a reason for hiding this comment

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

This one too require multiprocessing? No

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry wrong place, I am speaking of the following test.

@ogrisel ogrisel merged commit 1c57c18 into joblib:master Jun 20, 2018
@ogrisel ogrisel deleted the thread-bomb-protection branch June 20, 2018 14:34
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Jul 28, 2018
* 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)
  ...
yarikoptic added a commit to yarikoptic/joblib that referenced this pull request Jul 28, 2018
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants