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

Optimization: don't re-verify unchanged files #715

Merged
merged 2 commits into from Apr 2, 2016
Merged

Optimization: don't re-verify unchanged files #715

merged 2 commits into from Apr 2, 2016

Conversation

@dcposch
Copy link
Contributor

dcposch commented Apr 2, 2016

Let the user specify known-good file modtimes. If the files modtime is at least
that old, then the file hasn't changed and does not need to be re-verified.

This is only valid in node when using FS backing storage, not in the browser

Let the user specify known-good file modtimes. If the files modtime is at least
that old, then the file hasn't changed and does not need to be re-verified.

This is only valid in node when using FS backing storage, not in the browser
@dcposch dcposch force-pushed the dc/modtime branch from 03947ee to 3756ae6 Apr 2, 2016
return fileModtimes[index] === self._fileModtimes[index]
}).reduce(function (a, b) {
return a && b
})

This comment has been minimized.

Copy link
@feross

feross Apr 2, 2016

Member

This will probably crash when there's only one file, since a default starting value was not specified for reduce.

This comment has been minimized.

Copy link
@feross

feross Apr 2, 2016

Member

I think arr.every() works better here.

@@ -5,12 +5,12 @@ module.exports = Torrent
var addrToIPPort = require('addr-to-ip-port')
var BitField = require('bitfield')
var ChunkStoreWriteStream = require('chunk-store-stream/write')
var cpus = require('cpus')

This comment has been minimized.

Copy link
@feross

feross Apr 2, 2016

Member

This needs to be removed from package.json

@@ -45,6 +45,8 @@ var PIPELINE_MAX_DURATION = 1
var RECHOKE_INTERVAL = 10000 // 10 seconds
var RECHOKE_OPTIMISTIC_DURATION = 2 // 30 seconds

var FILESYSTEM_CONCURRENCY = 2

This comment has been minimized.

Copy link
@feross

feross Apr 2, 2016

Member

Why 2?

for (var index = 0; index < self.pieces.length; index++) {
self._markVerified(index)
}
self._onStore()

This comment has been minimized.

Copy link
@feross

feross Apr 2, 2016

Member

This could be optimized further to handle the case where only some files are modified, i.e. mark pieces that are wholly-contained within unchanged files as verified, and reverify the rest.

@feross

This comment has been minimized.

Copy link
Member

feross commented Apr 2, 2016

Thanks for the PR, @dcposch! Looks great.

@feross feross merged commit 84052ad into master Apr 2, 2016
2 of 4 checks passed
2 of 4 checks passed
continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@feross feross deleted the dc/modtime branch Apr 2, 2016
feross added a commit that referenced this pull request Apr 2, 2016
feross added a commit that referenced this pull request Apr 6, 2016
Fixes regression introduced in #715
@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

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