Skip to content

MAINT: Delayed imports to reduce import time #2809

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

Merged
merged 7 commits into from
Dec 8, 2018

Conversation

effigies
Copy link
Member

@effigies effigies commented Nov 28, 2018

Summary

This eliminates the import-time loading of:

  • scipy
  • networkx
  • bids (takes pandas with it)
  • paramiko
  • boto

Some possibly profitable additional excisions:

  • numpy
  • nibabel
  • requests

This reduces the profile to:

numpy: 136 modules; 75.44ms, 115.6MiB
nibabel: 271 modules; 392.09ms, 24.0MiB
requests: 283 modules; 105.75ms, 8.9MiB
traits: 36 modules; 20.46ms, 1.3MiB
nipype: 80 modules; 80.59ms, 4.8MiB
Final: 806 modules; 674.33ms, 154.7MiB
{'_json',
 'dateutil',
 'dateutil._common',
 'dateutil._version',
 'dateutil.parser',
 'dateutil.parser._parser',
 'dateutil.parser.isoparser',
 'dateutil.relativedelta',
 'dateutil.tz',
 'dateutil.tz._common',
 'dateutil.tz._factories',
 'dateutil.tz.tz',
 'duecredit',
 'duecredit.collector',
 'duecredit.config',
 'duecredit.dueswitch',
 'duecredit.entries',
 'duecredit.io',
 'duecredit.log',
 'duecredit.stub',
 'duecredit.utils',
 'duecredit.version',
 'duecredit.versions',
 'faulthandler',
 'fcntl',
 'future',
 'future.standard_library',
 'future.utils',
 'json',
 'json.decoder',
 'json.encoder',
 'json.scanner',
 'logging.handlers',
 'nipype',
 'nipype.external',
 'nipype.external.cloghandler',
 'nipype.external.due',
 'nipype.external.portalocker',
 'nipype.info',
 'nipype.interfaces',
 'nipype.interfaces.base',
 'nipype.interfaces.base.core',
 'nipype.interfaces.base.specs',
 'nipype.interfaces.base.support',
 'nipype.interfaces.base.traits_extension',
 'nipype.interfaces.io',
 'nipype.interfaces.utility',
 'nipype.interfaces.utility.base',
 'nipype.interfaces.utility.csv',
 'nipype.interfaces.utility.wrappers',
 'nipype.pipeline',
 'nipype.pipeline.engine',
 'nipype.pipeline.engine.base',
 'nipype.pipeline.engine.nodes',
 'nipype.pipeline.engine.utils',
 'nipype.pipeline.engine.workflows',
 'nipype.pkg_info',
 'nipype.refs',
 'nipype.utils',
 'nipype.utils.config',
 'nipype.utils.filemanip',
 'nipype.utils.functions',
 'nipype.utils.logger',
 'nipype.utils.misc',
 'nipype.utils.onetime',
 'nipype.utils.provenance',
 'nipype.utils.subprocess',
 'nipype.utils.tmpdirs',
 'packaging',
 'packaging.__about__',
 'packaging._structures',
 'packaging.version',
 'prov',
 'prov.constants',
 'prov.identifier',
 'prov.model',
 'prov.serializers',
 'shlex',
 'six.moves.urllib',
 'six.moves.urllib.parse'}

Related: #2776.

List of changes proposed in this PR (pull-request)

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

@effigies effigies changed the title MAINT: Lazy imports [WIP] MAINT: Lazy imports Nov 28, 2018
@effigies
Copy link
Member Author

@oesteban Would you be interested in running your fingerprinting on this branch, to see how much effect this has? That will help determine if it's worth going further.

@oesteban
Copy link
Contributor

Sure

@effigies
Copy link
Member Author

Auto-loading requests will go away with the next duecredit release including duecredit/duecredit#140, making duecredit a much lighter import

@oesteban
Copy link
Contributor

Not a big dent

Memory usage at entering main - RSS 122.6 MB / VM 846 MB
Object usage:
function                   31325
dict                       15452
tuple                      13881
list                       5726
weakref                    5429
cell                       5351
getset_descriptor          4125
type                       3434
builtin_function_or_method 3215
wrapper_descriptor         2948

Memory usage after parsing arguments - RSS 123.6 MB / VM 848 MB
Object usage:
function                   31326
dict                       15518
tuple                      13119
list                       5812
weakref                    5429
cell                       5351
getset_descriptor          4125
type                       3434
builtin_function_or_method 3215
wrapper_descriptor         2948

Memory usage before plugin initialization - RSS 125.0 MB / VM 849 MB
Object usage:
function                   31799
dict                       15769
tuple                      13447
list                       5875
weakref                    5651
cell                       5387
getset_descriptor          4182
type                       3530
builtin_function_or_method 3334
wrapper_descriptor         2948

Memory usage after plugin initialization - RSS 125.4 MB / VM 1041 MB
Object usage:
function                   31887
dict                       15861
tuple                      13497
list                       5897
weakref                    5691
cell                       5388
getset_descriptor          4196
type                       3543
builtin_function_or_method 3349
wrapper_descriptor         2948

Memory usage before sending workflow initialization to worker - RSS 125.4 MB / VM 1041 MB
Object usage:
function                   31887
dict                       15861
tuple                      13497
list                       5897
weakref                    5691
cell                       5388
getset_descriptor          4196
type                       3543
builtin_function_or_method 3349
wrapper_descriptor         2948

@codecov-io
Copy link

codecov-io commented Nov 29, 2018

Codecov Report

Merging #2809 into master will decrease coverage by 0.02%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2809      +/-   ##
==========================================
- Coverage   67.51%   67.48%   -0.03%     
==========================================
  Files         341      341              
  Lines       43316    43331      +15     
  Branches     5372     5371       -1     
==========================================
  Hits        29244    29244              
- Misses      13372    13388      +16     
+ Partials      700      699       -1
Flag Coverage Δ
#smoketests 50.55% <38.2%> (-0.09%) ⬇️
#unittests 64.9% <62.62%> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/cmtk/nx.py 17.94% <0%> (-0.31%) ⬇️
nipype/interfaces/spm/base.py 87.12% <0%> (-0.34%) ⬇️
nipype/interfaces/cmtk/cmtk.py 18.45% <0%> (-0.2%) ⬇️
nipype/interfaces/utility/base.py 87.5% <0%> (-0.57%) ⬇️
nipype/interfaces/spm/model.py 42.61% <0%> (-0.35%) ⬇️
nipype/pipeline/plugins/base.py 60.32% <100%> (+0.76%) ⬆️
nipype/pipeline/engine/utils.py 82.26% <100%> (+0.11%) ⬆️
nipype/algorithms/modelgen.py 67.44% <100%> (ø) ⬆️
nipype/algorithms/confounds.py 67.25% <100%> (-0.07%) ⬇️
nipype/algorithms/icc.py 59.21% <100%> (ø) ⬆️
... and 11 more

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 9f0ca09...5c4e266. Read the comment docs.

@oesteban
Copy link
Contributor

I guess this need to come accompanied by similar actions on fmriprep. BTW, I also used current master of duecredit.

@effigies
Copy link
Member Author

It's not really the import of niworkflows/fmriprep that's getting us.

numpy: 136 modules; 85.95ms, 115.6MiB
nibabel: 271 modules; 1253.36ms, 23.0MiB
duecredit: 13 modules; 6.16ms, 0.5MiB
traits: 36 modules; 23.28ms, 1.1MiB
nipype: 76 modules; 104.09ms, 3.4MiB
niworkflows: 32 modules; 52.32ms, 4.3MiB
fmriprep: 3 modules; 1.11ms, 0.0MiB
Final: 567 modules; 1526.28ms, 147.8MiB
{'fmriprep', 'fmriprep._version', 'fmriprep.info'}

@effigies
Copy link
Member Author

I don't know if this is the only impediment, but we can't confine numpy at all, due to the existence of the NUMPY_MMAP constant.

@effigies
Copy link
Member Author

So it's not looking like this is going to save us a bunch of memory at run time, but at least it can make startup a little snappier. Merge, if you think it's useful.

@effigies effigies changed the title [WIP] MAINT: Lazy imports MAINT: Lazy imports Nov 29, 2018
@effigies effigies changed the title MAINT: Lazy imports MAINT: Delayed imports to reduce import time Nov 29, 2018
@effigies
Copy link
Member Author

effigies commented Dec 6, 2018

I'd like to merge this... the load time of nipype is starting to grate in other contexts. If anybody has time to review this, I'd appreciate a second set of eyes. I just looked through again and caught a couple things.

@effigies effigies merged commit 1eeabd3 into nipy:master Dec 8, 2018
@effigies effigies deleted the maint/lazy_imports branch December 8, 2018 17:26
@effigies effigies added this to the 1.1.7 milestone Dec 14, 2018
yarikoptic added a commit to yarikoptic/nipype that referenced this pull request Jan 11, 2019
1.1.7 (December 17, 2018)

* FIX: Copy node list before generating a flat graph (nipy#2828)
* FIX: Update pytest req'd version to 3.6 (nipy#2827)
* FIX: Set ResourceMonitor.fname to an absolute path (nipy#2824)
* FIX: Order of SPM.NewSegment channel_info boolean tuple is (Field, Corrected) (nipy#2817)
* FIX: Indices were swapped for memory and cpu profile data (nipy#2816)
* FIX: ``status_callback`` not called with ``stop_on_first_crash`` (nipy#2810)
* FIX: Change undefined ScriptError on LFS plugin to IOError (nipy#2803)
* ENH: Add NaN failure mode to CompCor interfaces (nipy#2819)
* ENH: Enable cnr_maps and residuals outputs for FSL eddy (nipy#2750)
* ENH: Improve ``str2bool`` + doctests (nipy#2807)
* TST: Improve py.test configuration of doctests (nipy#2802)
* DOC: Update DOI badge to point to all versions (nipy#2804)
* MAINT: Offload interfaces with help formatting (nipy#2797)
* MAINT: Reduce minimal code redundancy in filemanip.get_dependencies (nipy#2782)
* MAINT: Delayed imports to reduce import time (nipy#2809)
...
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.

3 participants