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

FIX avoid spawning resource trackers on import #893

Merged
merged 8 commits into from
Jun 18, 2019

Conversation

tomMoral
Copy link
Contributor

Change the way we check for semaphore availability to avoid spawning resource trackers on joblib import.

I rely directly on SemLock for unix and did not change the bahavior on windows as there is no semaphore tracker in this case.

closes #841

@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #893 into master will increase coverage by 0.3%.
The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #893     +/-   ##
=========================================
+ Coverage   95.15%   95.46%   +0.3%     
=========================================
  Files          45       45             
  Lines        6440     6459     +19     
=========================================
+ Hits         6128     6166     +38     
+ Misses        312      293     -19
Impacted Files Coverage Δ
joblib/test/test_module.py 100% <100%> (ø) ⬆️
joblib/_compat.py 100% <100%> (ø) ⬆️
joblib/_multiprocessing_helpers.py 85.29% <93.33%> (+4.52%) ⬆️
joblib/disk.py 81.66% <0%> (-6.67%) ⬇️
joblib/test/test_numpy_pickle.py 98.35% <0%> (+0.65%) ⬆️
joblib/_parallel_backends.py 96.4% <0%> (+0.8%) ⬆️
joblib/test/test_parallel.py 96.96% <0%> (+1.1%) ⬆️
joblib/func_inspect.py 95.45% <0%> (+3.4%) ⬆️
joblib/testing.py 95% <0%> (+7.5%) ⬆️

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 0957e31...6ec71a4. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #893 into master will increase coverage by 0.41%.
The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
+ Coverage   95.15%   95.57%   +0.41%     
==========================================
  Files          45       45              
  Lines        6440     6459      +19     
==========================================
+ Hits         6128     6173      +45     
+ Misses        312      286      -26
Impacted Files Coverage Δ
joblib/test/test_module.py 100% <100%> (ø) ⬆️
joblib/_compat.py 100% <100%> (ø) ⬆️
joblib/_multiprocessing_helpers.py 85.29% <93.33%> (+4.52%) ⬆️
joblib/logger.py 86.84% <0%> (-1.32%) ⬇️
joblib/test/test_numpy_pickle.py 98.35% <0%> (+0.65%) ⬆️
joblib/test/test_parallel.py 96.96% <0%> (+1.1%) ⬆️
joblib/_parallel_backends.py 97.2% <0%> (+1.6%) ⬆️
joblib/func_inspect.py 95.45% <0%> (+3.4%) ⬆️
joblib/test/test_store_backends.py 97.05% <0%> (+5.88%) ⬆️
joblib/testing.py 95% <0%> (+7.5%) ⬆️

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 0957e31...6c69dca. Read the comment docs.

@tomMoral tomMoral force-pushed the FIX_no_early_semtracker branch from 6c69dca to 20ed298 Compare June 13, 2019 15:04
Copy link
Contributor

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM but a few quick comments.

Please add a changelog entry as well.

joblib/_compat.py Show resolved Hide resolved
joblib/_multiprocessing_helpers.py Outdated Show resolved Hide resolved
joblib/_multiprocessing_helpers.py Outdated Show resolved Hide resolved
joblib/_multiprocessing_helpers.py Outdated Show resolved Hide resolved
@ogrisel ogrisel merged commit aa17573 into joblib:master Jun 18, 2019
@ogrisel
Copy link
Contributor

ogrisel commented Jun 18, 2019

I squashed merged but i forgot about the misding entry in the changelog.

@tomMoral tomMoral deleted the FIX_no_early_semtracker branch June 19, 2019 07:18
@aclowes
Copy link

aclowes commented Jul 26, 2019

Thanks for fixing this!

@aclowes
Copy link

aclowes commented Jul 31, 2019

I'm interested in using this fix, is there anything I can do to speed the release of joblib 1.4.0 on pypi? Thanks!

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 31, 2019
Release 0.14.0
Improved the load balancing between workers to avoid stranglers caused by an excessively large batch size when the task duration is varying significantly (because of the combined use of joblib.Parallel and joblib.Memory with a partially warmed cache for instance). joblib/joblib#899
Add official support for Python 3.8: fixed protocol number in Hasher and updated tests.
Fix a deadlock when using the dask backend (when scattering large numpy arrays). joblib/joblib#914
Warn users that they should never use joblib.load with files from untrusted sources. Fix security related API change introduced in numpy 1.6.3 that would prevent using joblib with recent numpy versions. joblib/joblib#879
Upgrade to cloudpickle 1.1.1 that add supports for the upcoming Python 3.8 release among other things. joblib/joblib#878
Fix semaphore availability checker to avoid spawning resource trackers on module import. joblib/joblib#893
Fix the oversubscription protection to only protect against nested Parallel calls. This allows joblib to be run in background threads. joblib/joblib#934
Fix ValueError (negative dimensions) when pickling large numpy arrays on Windows. joblib/joblib#920
Upgrade to loky 2.6.0 that add supports for the setting environment variables in child before loading any module. joblib/joblib#940
Fix the oversubscription protection for native libraries using threadpools (OpenBLAS, MKL, Blis and OpenMP runtimes). The maximal number of threads is can now be set in children using the inner_max_num_threads in parallel_backend. It defaults to cpu_count() // n_jobs.
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.

joblib creates a semaphore at import time, resulting in spawning semaphore tracker
4 participants