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

Use thread pool for faster network IO #14

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pavdmyt
Copy link
Contributor

@pavdmyt pavdmyt commented Jun 13, 2017

Fetching package version info from PyPI can be much more faster by utilizing multiprocessing.dummy. Basically it's a wrapper around threading module which replicates multiprocessing API. It provides a convenient means to concurrently instantiate pypiup.requirement.Requirement objects which in turn make GET requests to PyPI. In short all code required for concurrent execution is only 4 lines:

# Setup pool of workers, qty of workers defaults to the number of CPU cores
pool = ThreadPool()
# Instantiate Requirement objects (and make GET requests) in their own threads
requirements = pool.map(Requirement, args)
# Close pool and wait all work to finish
pool.close()
pool.join()

Another part is to make progressbar work correctly in a concurrent mode. I've rewrote progressbar handling code a bit to manually update it's state. Basically click.progressbar setup step requires length param instead of iterable and update method should be called on bar each time we want to actually update it. To make sure that bar.update is called only after Requirement object instantiated and decouple this logic from Requirement class, I've implemented update functionality using barupdate decorator which automatically calls bar.update each time new Requirement object created.

Performance results

Below some results obtained by calculating time required for pypiup.requirements.Requirements.read_file execution:

Before:
-------

$ pypiup -r requirements/requirements-demo.txt  # (Total Requirements: 8)
read_file: 6.328423023223877

$ pypiup -r requirements-huge.txt  # (Total Requirements: 45)
read_file: 14.644872188568115

After:
------

$ pypiup -r requirements/requirements-demo.txt  # (Total Requirements: 8)
read_file: 0.9168651103973389

$ pypiup -r requirements-huge.txt  # (Total Requirements: 45)
read_file: 1.940899133682251

@codecov-io
Copy link

codecov-io commented Jun 13, 2017

Codecov Report

Merging #14 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
+ Coverage   99.18%   99.24%   +0.06%     
==========================================
  Files           4        4              
  Lines         123      133      +10     
==========================================
+ Hits          122      132      +10     
  Misses          1        1
Impacted Files Coverage Δ
pypiup/requirements.py 100% <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 53dfd92...2cb6abd. Read the comment docs.

@pavdmyt
Copy link
Contributor Author

pavdmyt commented Jul 6, 2017

@manosim Any chance we can get this merged, or a similar PR?

@pavdmyt
Copy link
Contributor Author

pavdmyt commented Jul 24, 2017

@manosim how about merging this PR as well? Sorry if pushing you, I'd like to have this in order to update my project's dev dependencies with newest pypiup ;)

@pavdmyt
Copy link
Contributor Author

pavdmyt commented Sep 19, 2017

@manosim the PR is up to date with master now. Can you merge it please?

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