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

VerifyPieces: Removed Parallel execution to prevent excessive memory consumption... #447

Closed
wants to merge 2 commits into from

Conversation

@ngjermundshaug
Copy link

ngjermundshaug commented Sep 28, 2015

…and also to prevent UI/Process to hang when verifying large files.

Also introduced a verifyingPieces flag on Torrent - in order to let consumers of the library detect when files are being verified.
self._onStore() is now being invoked before Torrent is verified - so that consumers can poll on the verifyingPieces flag.

UI Demo: http://screencast.com/t/NgbUPvQLik1O

…d also to prevent UI/Process to hang when verifying large files.

Also introduced a verifyingPieces flag on Torrent - in order to let consumers of the library detect when files are being verified.
self._onStore() is being invoked before Torrent is verified - so that consumers can poll on the verifyingPieces flag to detect this.
@josephfrazier

This comment has been minimized.

Copy link
Member

josephfrazier commented Oct 6, 2015

It looks like this change causes the travis builds to stall, and they also stall on my local machine. I know you said the tests won't run on Windows, but you could try rebasing your changes onto master (or merging master into your branch) and then testing locally? There were some recent changes that aim to allow the tests to run on Windows.

@IngoValente

This comment has been minimized.

Copy link

IngoValente commented Nov 30, 2015

Thanks for this, indeed keeps my UI usable while verifying.

However the sha1 hash check is async, so shouldn't the 'verify next piece check' be inside the sha1 callback function? I got stuck using your code because it would mark verifying as done before the last piece was actually verified, and that fixed it for me.

feross added a commit that referenced this pull request Jan 11, 2016
@feross

This comment has been minimized.

Copy link
Member

feross commented Jan 11, 2016

@ngjermundshaug @josephfrazier @IngoValente Thanks all, for your contributions to this PR!

The simplest – and fastest – solution is to limit the number of concurrent verifications to one per CPU core so we go as fast as possible, but no faster.

Here's what I decided to go with: 664eb30

@feross feross closed this Jan 11, 2016
@feross

This comment has been minimized.

Copy link
Member

feross commented Jan 11, 2016

Released as 0.72.1.

@lock

This comment has been minimized.

Copy link

lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.

@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.