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

ENH: Delay import of requests #140

Merged
merged 1 commit into from Nov 28, 2018

Conversation

Projects
None yet
4 participants
@effigies
Contributor

effigies commented Nov 28, 2018

Packages that load duecredit pull in 326 modules, 312 of which are loaded with requests.

This change delays import of requests to the single function that requests is called from.

ENH: Delay import of requests
Packages that load duecredit pull in 326 modules, 312 of which are loaded with requests.

This change delays import of requests to the single function that requests is called from.
@coveralls

This comment has been minimized.

coveralls commented Nov 28, 2018

Coverage Status

Coverage decreased (-3.5%) to 86.171% when pulling 8f67c98 on effigies:patch-2 into 571fae5 on duecredit:master.

@codecov-io

This comment has been minimized.

codecov-io commented Nov 28, 2018

Codecov Report

Merging #140 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #140   +/-   ##
=======================================
  Coverage   83.21%   83.21%           
=======================================
  Files          44       44           
  Lines        2306     2306           
  Branches      251      251           
=======================================
  Hits         1919     1919           
  Misses        316      316           
  Partials       71       71
Impacted Files Coverage Δ
duecredit/io.py 82% <100%> (ø) ⬆️

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 571fae5...8f67c98. Read the comment docs.

@yarikoptic

This comment has been minimized.

Member

yarikoptic commented Nov 28, 2018

THANK YOU! Heavy dependencies are indeed one of the cons of duecredit in general, we should look into lightning it up

@yarikoptic yarikoptic merged commit 36022a9 into duecredit:master Nov 28, 2018

4 of 5 checks passed

coverage/coveralls Coverage decreased (-3.5%) to 86.171%
Details
codecov/patch 100% of diff hit (target 83.21%)
Details
codecov/project 83.21% (+0%) compared to 571fae5
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@effigies effigies deleted the effigies:patch-2 branch Nov 28, 2018

@effigies

This comment has been minimized.

Contributor

effigies commented Nov 28, 2018

Following this change, here's the difference in number of loaded modules, timestamp and virtual memory size after loading duecredit (sys, psutil, time and importlib are the only things imported first):

duecredit: 37 modules; 36.47ms, 3.6MiB

If I load numpy before duecredit (which will be common), the increment is:

duecredit: 17 modules; 36.55ms, 0.0MiB

@effigies effigies referenced this pull request Nov 28, 2018

Merged

MAINT: Delayed imports to reduce import time #2809

7 of 9 tasks complete
@yarikoptic

This comment has been minimized.

Member

yarikoptic commented Nov 29, 2018

See ; it is better to load duecredit first - much faster! 36ms and 3MB is just a spit in a bucket ... What do you get for import of pkg_resources?

@yarikoptic

This comment has been minimized.

Member

yarikoptic commented Nov 29, 2018

I might add later a test to make sure that we don't bring 💯 s of modules import back. Kept fighting similar incidents in datalad to keep its start time relatively low

@effigies

This comment has been minimized.

Contributor

effigies commented Nov 29, 2018

pkg_resources: 72 modules; 282.91ms, 7.9MiB

@yarikoptic

This comment has been minimized.

Member

yarikoptic commented Nov 29, 2018

here you go x8 times longer ;-) btw - what do you use for those measurements or just manually timing/comparing prior/after?

@yarikoptic

This comment has been minimized.

Member

yarikoptic commented Nov 29, 2018

may be ever come handy -- we have a little https://github.com/datalad/datalad/blob/master/tools/dtime helper to use along with logging, so you can see how long it took from previous line to the next one, and then can sort... example:

$> DATALAD_LOG_TIMESTAMP=1 datalad -l 1 --help 2>&1 | ~datalad/datalad/tools/dtime | head
    0 2018-11-28 21:39:08,007 [DEBUG] Command line args 1st pass. Parsed: Namespace() Unparsed: ['--help'] 
    1 2018-11-28 21:39:08,007 [DEBUG] Discovering plugins 
  399 2018-11-28 21:39:08,008 [DEBUG] Loading entrypoints 
    1 2018-11-28 21:39:08,407 [Level 5] Importing module datalad.distribution.create  
    4 2018-11-28 21:39:08,408 [DEBUG] Building doc for <class 'datalad.interface.annotate_paths.AnnotatePaths'> 
    2 2018-11-28 21:39:08,412 [DEBUG] Building doc for <class 'datalad.interface.save.Save'> 
    3 2018-11-28 21:39:08,414 [DEBUG] Building doc for <class 'datalad.distribution.add.Add'> 
    2 2018-11-28 21:39:08,417 [DEBUG] Building doc for <class 'datalad.distribution.subdatasets.Subdatasets'> 
    6 2018-11-28 21:39:08,419 [DEBUG] Building doc for <class 'datalad.distribution.create.Create'> 
    1 2018-11-28 21:39:08,425 [Level 5] Importing module datalad.distribution.install  
...

so you can see what hurts us most when "exhaustive" --help (we need to discover all the entry points) is invoked.

@effigies

This comment has been minimized.

Contributor

effigies commented Nov 29, 2018

https://gist.github.com/effigies/183898aa6ef047eb8ef86f7e63f5528a#file-import_profile-py

Just modify the sequential list of modules to load. (sys is there as a dummy to make the logic simpler.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment