fix: #39 - Improve constructor performance (#45)

package.json

@@ -43,7 +43,8 @@
     "aegir": "^12.0.8",
     "chai": "^4.1.2",
     "dirty-chai": "^2.0.1",
-    "multihashing-async": "~0.4.6",
+    "multihashing": "^0.3.2",
+    "multihashing-async": "^0.4.8",
     "pre-commit": "^1.2.2"
   },
   "engines": {

src/cid-util.js

@@ -0,0 +1,43 @@
+'use strict'
+
+const mh = require('multihashes')
+
+var CIDUtil = {
+  /**
+   * Test if the given input is a valid CID object.
+   * Returns an error message if it is not.
+   * Returns undefined if it is a valid CID.
+   *
+   * @param {any} other
+   * @returns {string}
+   */
+  checkCIDComponents: function (other) {
+    if (other == null) {
+      return 'null values are not valid CIDs'
+    }
+
+    if (!(other.version === 0 || other.version === 1)) {
+      return 'Invalid version, must be a number equal to 1 or 0'
+    }
+
+    if (typeof other.codec !== 'string') {
+      return 'codec must be string'
+    }
+
+    if (!Buffer.isBuffer(other.multihash)) {
+      return 'multihash must be a Buffer'
+    }
+
+    try {
+      mh.validate(other.multihash)
+    } catch (err) {
+      let errorMsg = err.message
+      if (!errorMsg) { // Just in case mh.validate() throws an error with empty error message
+        errorMsg = 'Multihash validation failed'
+      }
+      return errorMsg
+    }
+  }
+}
+
+module.exports = CIDUtil

src/index.js

@@ -6,6 +6,7 @@ const multicodec = require('multicodec')
 const codecs = require('multicodec/src/base-table')
 const codecVarints = require('multicodec/src/varint-table')
 const multihash = require('multihashes')
+const CIDUtil = require('./cid-util')
 
 /**
  * @typedef {Object} SerializedCID
@@ -218,13 +219,7 @@ class CID {
    * @returns {bool}
    */
   static isCID (other) {
-    try {
-      CID.validateCID(other)
-    } catch (err) {
-      return false
-    }
-
-    return true
+    return !(CIDUtil.checkCIDComponents(other))
   }
 
   /**
@@ -235,23 +230,10 @@ class CID {
    * @returns {void}
    */
   static validateCID (other) {
-    if (other == null) {
-      throw new Error('null values are not valid CIDs')
+    let errorMsg = CIDUtil.checkCIDComponents(other)
+    if (errorMsg) {
+      throw new Error(errorMsg)
     }
-
-    if (!(other.version === 0 || other.version === 1)) {
-      throw new Error('Invalid version, must be a number equal to 1 or 0')
-    }
-
-    if (typeof other.codec !== 'string') {
-      throw new Error('codec must be string')
-    }
-
-    if (!Buffer.isBuffer(other.multihash)) {
-      throw new Error('multihash must be a Buffer')
-    }
-
-    mh.validate(other.multihash)
   }
 }
 

test/cid-util.spec.js

@@ -0,0 +1,75 @@
+/* eslint-env mocha */
+/* eslint max-nested-callbacks: ["error", 8] */
+'use strict'
+
+const chai = require('chai')
+const dirtyChai = require('dirty-chai')
+const expect = chai.expect
+chai.use(dirtyChai)
+const multihashing = require('multihashing-async')
+const CID = require('../src')
+const CIDUtil = require('../src/cid-util')
+
+describe('CIDUtil', () => {
+  let hash
+
+  before((done) => {
+    multihashing(Buffer.from('abc'), 'sha2-256', (err, d) => {
+      if (err) {
+        return done(err)
+      }
+      hash = d
+      done()
+    })
+  })
+
+  describe('checkCIDComponents()', () => {
+    describe('returns undefined on valid CID', () => {
+      it('create from B58Str multihash', () => {
+        expect(() => {
+          const mhStr = 'QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n'
+          const cid = new CID(mhStr)
+          const errMsg = CIDUtil.checkCIDComponents(cid)
+          expect(errMsg).to.not.exist()
+        }).to.not.throw()
+      })
+      it('create by parts', () => {
+        expect(() => {
+          const cid = new CID(1, 'dag-cbor', hash)
+          const errMsg = CIDUtil.checkCIDComponents(cid)
+          expect(errMsg).to.not.exist()
+        }).to.not.throw()
+      })
+    })
+
+    describe('returns non-null error message on invalid inputs', () => {
+      const invalid = [
+        'hello world',
+        'QmaozNR7DZHQK1ZcU9p7QdrshMvXqWK6gpu5rmrkPdT3L',
+        Buffer.from('hello world'),
+        Buffer.from('QmaozNR7DZHQK1ZcU9p7QdrshMvXqWK6gpu5rmrkPdT')
+      ]
+
+      invalid.forEach((i) => it(`new CID(${Buffer.isBuffer(i) ? 'buffer' : 'string'}<${i.toString()}>)`, () => {
+        expect(() => {
+          const errMsg = CIDUtil.checkCIDComponents(i)
+          expect(errMsg).to.exist()
+        }).to.not.throw()
+      }))
+
+      invalid.forEach((i) => it(`new CID(0, 'dag-pb', ${Buffer.isBuffer(i) ? 'buffer' : 'string'}<${i.toString()}>)`, () => {
+        expect(() => {
+          const errMsg = CIDUtil.checkCIDComponents(0, 'dag-pb', i)
+          expect(errMsg).to.exist()
+        }).to.not.throw()
+      }))
+
+      invalid.forEach((i) => it(`new CID(1, 'dag-pb', ${Buffer.isBuffer(i) ? 'buffer' : 'string'}<${i.toString()}>)`, () => {
+        expect(() => {
+          const errMsg = CIDUtil.checkCIDComponents(1, 'dag-pb', i)
+          expect(errMsg).to.exist()
+        }).to.not.throw()
+      }))
+    })
+  })
+})

test/profiling/cidperf-x.js

@@ -0,0 +1,52 @@
+'use strict'
+
+const multihashing = require('multihashing')
+// [1] Original/existing implementation.
+// const CID = require('cids')
+// [2] New/proposed implementation.
+const CID = require('../../src')
+
+// Used to delay the testing for a few seconds.
+function sleep (ms) {
+  return new Promise(resolve => setTimeout(resolve, ms))
+}
+
+// Simple test class.
+// The purpose of this class is
+// to simply test the CID ctor (and, primarily, CID.isCID() method).
+class CIDPerfX {
+  constructor () {
+    this.version = 1
+    this.codec = 'dag-pb'
+    this.mh = multihashing(Buffer.from('oh, hey!'), 'sha2-256')
+  }
+
+  // i: Running-counter.
+  // print: If true, it'll print/dump the CID data.
+  run (i, print) {
+    const cid = new CID(this.version, this.codec, this.mh)
+    if (print === true) {
+      console.log('i=' + i, cid)
+    }
+  }
+}
+
+// /////////////////////////
+// Main test routine.
+// Note: You will need to run the test separately
+//       for each "alternative" impls and compare their results.
+// /////////////////////////
+
+const reps = 10000
+const cidPerf = new CIDPerfX()
+
+console.log(`Test: Will run "new CID()" ${reps} times.`)
+// We just give ~1 second for the JS engine to start and 'rest', etc.
+// before starting new tests.
+sleep(1000).then(() => {
+  console.log(`Starting a test...`)
+  console.time('run'); [...Array(reps).keys()].map(i => {
+    cidPerf.run(i)
+  })
+  console.timeEnd('run')
+})