Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Commit

Permalink
perf: make this._json calculation lazy
Browse files Browse the repository at this point in the history
I've been doing some profiling of js-ipfs and where there are large amounts
of DAG operations being performed (importing all of npm to ipfs, for
example) we spend about 1.8-2.5% of our time in the creation of this._json
objects, even though they are not always used, so this PR makes them be
created when first requested.

It also passes the `this._json` object through `Object.freeze` becase otherwise
we expose a mutable object to the world - it seems odd that we've taken pains
to ensure that the properties of a DAGNode/DAGLink are immutable, then expose
objects whose properties are not immutable.

Finally it makes more use of the `cids` module to parse CIDs from multihashes
as the `cids` module respects CIDv1 versions/codecs if present whereas the
`multihashes` module does not and is only really suitable for use with CIDv0.
  • Loading branch information
achingbrain authored and vmx committed Aug 13, 2018
1 parent 97bbf30 commit d138c95
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 40 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
"cids": "~0.5.3",
"class-is": "^1.1.0",
"is-ipfs": "~0.4.2",
"multihashes": "~0.4.13",
"multihashing-async": "~0.5.1",
"protons": "^1.0.1",
"pull-stream": "^3.6.8",
Expand All @@ -78,6 +77,7 @@
"ipfs-block-service": "~0.14.0",
"ipfs-repo": "~0.22.1",
"lodash": "^4.17.10",
"multihashes": "~0.4.13",
"ncp": "^2.0.0",
"rimraf": "^2.6.2"
}
Expand Down
26 changes: 11 additions & 15 deletions src/dag-link/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,23 @@ class DAGLink {

this._name = name || ''
this._size = size

if (typeof multihash === 'string') {
const cid = new CID(multihash)
this._multihash = cid.buffer
} else if (Buffer.isBuffer(multihash)) {
this._multihash = multihash
}
this._cid = new CID(multihash)
}

toString () {
const cid = new CID(this.multihash)
return `DAGLink <${cid.toBaseEncodedString()} - name: "${this.name}", size: ${this.size}>`
return `DAGLink <${this._cid.toBaseEncodedString()} - name: "${this.name}", size: ${this.size}>`
}

toJSON () {
const cid = new CID(this.multihash)
return {
name: this.name,
size: this.size,
multihash: cid.toBaseEncodedString()
if (!this._json) {
this._json = Object.freeze({
name: this.name,
size: this.size,
multihash: this._cid.toBaseEncodedString()
})
}

return this._json
}

get name () {
Expand All @@ -54,7 +50,7 @@ class DAGLink {
}

get multihash () {
return this._multihash
return this._cid.buffer
}

set multihash (multihash) {
Expand Down
3 changes: 1 addition & 2 deletions src/dag-node/addLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ function addLink (node, link, callback) {
link = toDAGLink(link)
} else {
// It's a Object with name, multihash/link and size
link.multihash = link.multihash || link.hash
try {
link = new DAGLink(link.name, link.size, link.multihash)
link = new DAGLink(link.name, link.size, link.multihash || link.hash)
} catch (err) {
return callback(err)
}
Expand Down
35 changes: 17 additions & 18 deletions src/dag-node/index.js
Original file line number Diff line number Diff line change
@@ -1,40 +1,35 @@
'use strict'

const mh = require('multihashes')
const assert = require('assert')
const withIs = require('class-is')
const CID = require('cids')

class DAGNode {
constructor (data, links, serialized, multihash) {
assert(serialized, 'DAGNode needs its serialized format')
assert(multihash, 'DAGNode needs its multihash')

if (typeof multihash === 'string') {
multihash = mh.fromB58String(multihash)
}

this._cid = new CID(multihash)
this._data = data || Buffer.alloc(0)
this._links = links || []
this._serialized = serialized
this._multihash = multihash

this._size = this.links.reduce((sum, l) => sum + l.size, this.serialized.length)

this._json = {
data: this.data,
links: this.links.map((l) => l.toJSON()),
multihash: mh.toB58String(this.multihash),
size: this.size
}
}

toJSON () {
if (!this._json) {
this._json = Object.freeze({
data: this.data,
links: this.links.map((l) => l.toJSON()),
multihash: this._cid.toBaseEncodedString(),
size: this.size
})
}

return this._json
}

toString () {
const mhStr = mh.toB58String(this.multihash)
return `DAGNode <${mhStr} - data: "${this.data.toString()}", links: ${this.links.length}, size: ${this.size}>`
return `DAGNode <${this._cid.toBaseEncodedString()} - data: "${this.data.toString()}", links: ${this.links.length}, size: ${this.size}>`
}

get data () {
Expand Down Expand Up @@ -62,6 +57,10 @@ class DAGNode {
}

get size () {
if (this._size === undefined) {
this._size = this.links.reduce((sum, l) => sum + l.size, this.serialized.length)
}

return this._size
}

Expand All @@ -70,7 +69,7 @@ class DAGNode {
}

get multihash () {
return this._multihash
return this._cid.buffer
}

set multihash (multihash) {
Expand Down
4 changes: 2 additions & 2 deletions test/dag-link-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)
const mh = require('multihashes')
const CID = require('cids')
const DAGLink = require('../src').DAGLink

module.exports = (repo) => {
Expand All @@ -26,7 +26,7 @@ module.exports = (repo) => {
it('create with multihash as a multihash Buffer', () => {
const link = new DAGLink('hello', 3, Buffer.from('12208ab7a6c5e74737878ac73863cb76739d15d4666de44e5756bf55a2f9e9ab5f43', 'hex'))

expect(mh.toB58String(link.multihash))
expect(new CID(link.multihash).toBaseEncodedString())
.to.equal('QmXg9Pp2ytZ14xgmQjYEiHjVjMFXzCVVEcRTWJBmLgR39U')
})

Expand Down
4 changes: 2 additions & 2 deletions test/dag-node-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ module.exports = (repo) => {
})
},
(cb) => {
const link = toDAGLink(node2).toJSON()
const link = Object.assign({}, toDAGLink(node2).toJSON())
link.name = 'banana'

DAGNode.addLink(node1a, link, (err, node) => {
Expand Down Expand Up @@ -402,7 +402,7 @@ module.exports = (repo) => {
})
},
(cb) => {
const link = toDAGLink(node2).toJSON()
const link = Object.assign({}, toDAGLink(node2).toJSON())
link.name = 'banana'

DAGNode.addLink(node1a, link, (err, node) => {
Expand Down

0 comments on commit d138c95

Please sign in to comment.