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

Speed up uploading/downloading of file and directories. #958

Merged
merged 2 commits into from
Aug 15, 2019

Conversation

serhiy-storchaka
Copy link
Contributor

Before optimization:
1 minute for neuro storage cp -r neuromation storage:
1 minute for neuro storage cp -r storage:neuromation /tmp

After optimization:
36 seconds for neuro storage cp -r neuromation storage:
34 seconds for neuro storage cp -r storage:neuromation /tmp

For comparison:
From 22 seconds to 1 minute 16 seconds for neuro storage load -r neuromation storage:
18 seconds for neuro storage load -r storage:neuromation /tmp
7 seconds for neuro storage cp neuromation.tar storage:
2 seconds for neuro storage cp storage:neuromation.tar /tmp

@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #958 into master will increase coverage by 0.01%.
The diff coverage is 92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #958      +/-   ##
==========================================
+ Coverage   92.08%   92.09%   +0.01%     
==========================================
  Files          37       37              
  Lines        4243     4251       +8     
  Branches      643      642       -1     
==========================================
+ Hits         3907     3915       +8     
  Misses        227      227              
  Partials      109      109
Impacted Files Coverage Δ
neuromation/api/storage.py 98.26% <92%> (+0.04%) ⬆️

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 cf1a4ca...00c23a7. Read the comment docs.

Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

LGTM.
The PR worth changelog record

@dalazx
Copy link
Contributor

dalazx commented Aug 15, 2019

@serhiy-storchaka curious, in case of load, what's the overhead of spawning a job, provided a local docker image is present already?

@asvetlov
Copy link
Contributor

what's the overhead of spawning a job, provided a local docker image is present already?

About 20 secs IIRC

@serhiy-storchaka serhiy-storchaka merged commit 078fd59 into master Aug 15, 2019
@serhiy-storchaka serhiy-storchaka deleted the storage-optimize branch August 15, 2019 10:44
@dalazx
Copy link
Contributor

dalazx commented Aug 15, 2019

18 seconds for neuro storage load -r storage:neuromation /tmp
does that include launching a job?

@serhiy-storchaka
Copy link
Contributor Author

Yes, it is a total time.

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

Successfully merging this pull request may close these issues.

4 participants