Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

fix: performance improvements #107

Merged
merged 20 commits into from
May 8, 2019
Merged

fix: performance improvements #107

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
12fb0d7
fix: add basic timeout to individual query func calls
jacobheun Apr 24, 2019
bf152e0
fix: per s/kademlia start with K peers, not Alpha
jacobheun Apr 24, 2019
81c26d9
fix: properly use kBucketSize option when provided
jacobheun Apr 24, 2019
57133a3
fix: make disjoint paths a function of kBucketSize/2
jacobheun Apr 24, 2019
6ce1e1b
refactor: split up query classes for easier testing
jacobheun Apr 25, 2019
a1f9e2b
test: add test to verify paths stop
jacobheun Apr 25, 2019
e49045d
chore: fix linting
jacobheun Apr 25, 2019
2d3bacd
fix: make alpha 3 again, reduce rpc message timeout
jacobheun Apr 25, 2019
81a558c
refactor: remove unneeded peerBook.put and update jsdocs
jacobheun Apr 25, 2019
75ec755
fix: stop the worker if it is done querying
jacobheun Apr 25, 2019
e21967f
test: add spy on continueQuering to its call count
jacobheun Apr 25, 2019
909f44c
docs: update jsdocs on utils
jacobheun Apr 25, 2019
a0eab2c
fix: make queries quicker but sloppier
jacobheun Apr 26, 2019
6e7ba6c
fix: dont error provide until all peers have been attempted
jacobheun Apr 26, 2019
d9831bb
fix: add timeout to query func
jacobheun Apr 26, 2019
61a8e26
test: add a simulation
jacobheun Apr 26, 2019
1e63a30
feat: allow configuration of alpha concurrency
jacobheun Apr 27, 2019
a1c9890
test(sim): add common peer check
jacobheun Apr 27, 2019
fa203c7
fix: apply suggestions from code review
dirkmc Apr 29, 2019
9c886f1
fix: remove ncp in favor of kBucketSize
jacobheun May 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ exports.K = 20
// Alpha is the concurrency for asynchronous requests
exports.ALPHA = 3

// How long individual query messages are allowed to take
dirkmc marked this conversation as resolved.
Show resolved Hide resolved
exports.MAX_MESSAGE_TIMEOUT = 5 * minute

// Number of disjoint query paths to use
// This is set to K/2 per the S/Kademlia paper
exports.DISJOINT_PATHS = 10
Expand Down
6 changes: 3 additions & 3 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ class KadDHT extends EventEmitter {
waterfall([
(cb) => utils.convertBuffer(key, cb),
(id, cb) => {
const rtp = this.routingTable.closestPeers(id, c.ALPHA)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing as this code is the same for every query, maybe it should be part of the Query itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

const rtp = this.routingTable.closestPeers(id, c.K)

this._log('peers in rt: %d', rtp.length)
if (rtp.length === 0) {
Expand Down Expand Up @@ -412,7 +412,7 @@ class KadDHT extends EventEmitter {
return callback(err)
}

const tablePeers = this.routingTable.closestPeers(id, c.ALPHA)
const tablePeers = this.routingTable.closestPeers(id, c.K)

const q = new Query(this, key, () => {
// There is no distinction between the disjoint paths,
Expand Down Expand Up @@ -613,7 +613,7 @@ class KadDHT extends EventEmitter {
waterfall([
(cb) => utils.convertPeerId(id, cb),
(key, cb) => {
const peers = this.routingTable.closestPeers(key, c.ALPHA)
const peers = this.routingTable.closestPeers(key, c.K)

if (peers.length === 0) {
return cb(errcode(new Error('Peer lookup failed'), 'ERR_LOOKUP_FAILED'))
Expand Down
2 changes: 1 addition & 1 deletion src/private.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ module.exports = (dht) => ({
}
})

const peers = dht.routingTable.closestPeers(key.buffer, c.ALPHA)
const peers = dht.routingTable.closestPeers(key.buffer, c.K)

timeout((cb) => query.run(peers, cb), providerTimeout)((err) => {
query.stop()
Expand Down
11 changes: 9 additions & 2 deletions src/query.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const EventEmitter = require('events')
const waterfall = require('async/waterfall')
const each = require('async/each')
const queue = require('async/queue')
const timeout = require('async/timeout')
const mh = require('multihashes')

const c = require('./constants')
Expand Down Expand Up @@ -81,6 +82,7 @@ class Query {
*/
_onStart () {
this.running = true
this._startTime = Date.now()
this._log('query:start')

// Register this query so we can stop it if the DHT stops
Expand All @@ -91,7 +93,7 @@ class Query {
* Called when the run completes (even if there's an error).
*/
_onComplete () {
this._log('query:done')
this._log(`query:done in ${Date.now() - this._startTime}ms`)

// Ensure worker queues for all paths are stopped at the end of the query
this.stop()
Expand Down Expand Up @@ -339,7 +341,7 @@ class Path {
*/
constructor (run, queryFunc) {
this.run = run
this.queryFunc = queryFunc
this.queryFunc = timeout(queryFunc, c.MAX_MESSAGE_TIMEOUT) // TODO: make configurable
this.initialPeers = []
}

Expand Down Expand Up @@ -456,6 +458,7 @@ class WorkerQueue {
* @param {Error} err
*/
stop (err) {
this.log('worker:stop')
if (!this.running) {
return
}
Expand Down Expand Up @@ -495,6 +498,10 @@ class WorkerQueue {
this.path.peersToQuery.length > 0) {
this.queue.push(this.path.peersToQuery.dequeue())
}

if (this.queue.length() < 1) {
this.log('queue is empty and cant be filled')
}
}

/**
Expand Down