Optimizations and refactoring #35
Merged
+120
−248
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the file download file name from content-disposition wouldn't be actually used, since d.name is set once during the first call to Name(), which is before the update using content-disposition. Now it works correctly (renamed d.name to d.path to signify that its the whole path)
Refactor
Remove redundant download when no range support is available, since we always download during init anyways.
Concurrently write directly to single file using WriteAt instead of writing to multiple files and then copying
Complete optimization
|
The optimizations done here (concurrently writing to single file) also fix what has been mentioned in #32 (slow writing to disk), since now the fastest way possible is used |
|
Also fixes all problems described in #29! |
|
|
|
it's fails only on 1.14.x windows, i think the tests gonna pass on 1.16 and 1.17 |
|
Thank you for your valuable pr |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi! Loved the idea of this tool, so I decided to contribute some improvements:
Not sure why, but the test workflows on go 1.14 windows are sometimes not passing (even though they are marked as ok, some unrelated errors are thrown by go vet, which makes the github CI workflow get marked as failed)
Didn't measure the optimizations on a server with good internet, but my local testing seems to save around 3+ seconds each time since we avoid file copying