Repository: pinojs/pino
Pull request #211 (2017-03-18 00:01:11)

Refactor construction to avoid `new`
I'm not sure if you guys want this. I just wanted to see how Pino would perform if it didn't rely on the `new` construct. I am attaching single runs of the full benchmark suite from the current master and this refactor branch. It would be nice to know how the runs compare on some other systems. I'll let you be the judge of the results.

There are a lot of changes here. You may want to simply view the full file instead of the diff. I started the work from the issue-209 branch in PR #210.

One nice change here, in my opinion, is that we don't have to expose the `_Pino` property for things like `pino-multi-stream`. Those sort of modules can simply get a new instance out of the factory function like normal and do true JavaScript "inheritance".

[refactor.bench.txt](https://github.com/pinojs/pino/files/852001/refactor.bench.txt)
[master.bench.txt](https://github.com/pinojs/pino/files/852002/master.bench.txt)





Patch:
diff --git a/pino.js b/pino.js
index 98c4361..46b5648 100644
--- a/pino.js
+++ b/pino.js
@@ -30,89 +30,27 @@ var defaultOptions = {
   enabled: true
 }
 
-function pino (opts, stream) {
-  var iopts = opts
-  var istream = stream
-  if (iopts && (iopts.writable || iopts._writableState)) {
-    istream = iopts
-    iopts = defaultOptions
-  }
-  iopts = Object.assign({}, defaultOptions, iopts)
-  if (iopts.extreme && iopts.prettyPrint) throw Error('cannot enable pretty print in extreme mode')
-  istream = istream || process.stdout
-  if (iopts.prettyPrint) {
-    var pstream = pretty(iopts.prettyPrint)
-    var origStream = istream
-    pump(pstream, origStream, function (err) {
-      if (err) logger.emit('error', err)
-    })
-    istream = pstream
-  }
-
-  // internal options
-  iopts.stringify = iopts.safe ? stringifySafe : JSON.stringify
-  iopts.formatOpts = {lowres: true}
-  iopts.end = ',"v":' + LOG_VERSION + '}\n'
-  iopts.cache = !iopts.extreme ? null : {
-    size: 4096,
-    buf: ''
-  }
-  iopts.chindings = ''
-
-  if (iopts.enabled === false) {
-    iopts.level = 'silent'
-  }
-
-  var settleTries = 0
-  function waitForFDSettle () {
-    var isBlockable = tools.streamIsBlockable(istream)
-    if (isBlockable === false && settleTries > 10) {
-      return logger.emit('error', Error('stream must have a file descriptor in extreme mode'))
-    } else if (isBlockable === true) {
-      return events(logger, extremeModeExitHandler)
-    }
-    settleTries += 1
-    setTimeout(waitForFDSettle, 100)
-  }
-
-  function extremeModeExitHandler () {
-    var buf = iopts.cache.buf
-    if (buf) {
-      // We need to block the process exit long enough to flush the buffer
-      // to the destination stream. We do that by forcing a synchronous
-      // write directly to the stream's file descriptor.
-      var fd = (istream.fd) ? istream.fd : istream._handle.fd
-      fs.writeSync(fd, buf)
-    }
+var pinoPrototype = Object.create(EventEmitter.prototype, {
+  silent: {
+    value: tools.noop,
+    enumerable: true
+  },
+  stream: {
+    value: process.stdout,
+    writable: true
   }
+})
 
-  var logger = new Pino(iopts, istream)
-  if (iopts.cache) setTimeout(waitForFDSettle, 100)
-
-  return logger
-}
-
-tools.defineLevelsProperty(pino)
-
-function Pino (opts, stream) {
-  // We define the levels property at construction so that state does
-  // not get shared between instances.
-  tools.defineLevelsProperty(this)
-  this.stream = stream
-  tools.applyOptions.call(this, opts)
-}
-
-Pino.prototype = new EventEmitter()
-
-Pino.prototype.fatal = tools.genLog(levels.levels.fatal)
-Pino.prototype.error = tools.genLog(levels.levels.error)
-Pino.prototype.warn = tools.genLog(levels.levels.warn)
-Pino.prototype.info = tools.genLog(levels.levels.info)
-Pino.prototype.debug = tools.genLog(levels.levels.debug)
-Pino.prototype.trace = tools.genLog(levels.levels.trace)
-Pino.prototype.silent = tools.noop
+var levelMethods = ['fatal', 'error', 'warn', 'info', 'debug', 'trace']
+levelMethods.forEach(function (m) {
+  Object.defineProperty(pinoPrototype, m, {
+    value: tools.genLog(levels.levels[m]),
+    enumerable: true,
+    writable: true
+  })
+})
 
-Object.defineProperty(Pino.prototype, 'levelVal', {
+Object.defineProperty(pinoPrototype, 'levelVal', {
   get: function getLevelVal () {
     return this._levelVal
   },
@@ -130,142 +68,228 @@ Object.defineProperty(Pino.prototype, 'levelVal', {
         this[key] = tools.noop
         continue
       }
-      this[key] = levels.isStandardLevel(key) ? Pino.prototype[key] : tools.genLog(this.levels.values[key])
+      this[key] = levels.isStandardLevel(key) ? pinoPrototype[key] : tools.genLog(this.levels.values[key])
     }
   }
 })
 
-Pino.prototype._setLevel = function _setLevel (level) {
-  if (typeof level === 'number') {
-    if (!isFinite(level)) {
+Object.defineProperty(pinoPrototype, '_setLevel', {
+  value: function _setLevel (level) {
+    if (typeof level === 'number') {
+      if (!isFinite(level)) {
+        throw Error('unknown level ' + level)
+      }
+      level = this.levels.labels[level]
+    }
+
+    if (!this.levels.values[level]) {
       throw Error('unknown level ' + level)
     }
-    level = this.levels.labels[level]
+    this.levelVal = this.levels.values[level]
   }
+})
 
-  if (!this.levels.values[level]) {
-    throw Error('unknown level ' + level)
+Object.defineProperty(pinoPrototype, '_getLevel', {
+  value: function _getLevel (level) {
+    return this.levels.labels[this.levelVal]
   }
-  this.levelVal = this.levels.values[level]
-}
-
-Pino.prototype._getLevel = function _getLevel (level) {
-  return this.levels.labels[this.levelVal]
-}
+})
 
-Object.defineProperty(Pino.prototype, 'level', {
-  get: Pino.prototype._getLevel,
-  set: Pino.prototype._setLevel
+Object.defineProperty(pinoPrototype, 'level', {
+  get: pinoPrototype._getLevel,
+  set: pinoPrototype._setLevel
 })
 
-Object.defineProperty(Pino.prototype, '_lscache', {
+Object.defineProperty(pinoPrototype, '_lscache', {
   value: tools.copy({}, levels.lscache)
 })
 
 Object.defineProperty(
-  Pino.prototype,
+  pinoPrototype,
   'LOG_VERSION',
   {value: LOG_VERSION}
 )
 
-Pino.prototype.asJson = function asJson (obj, msg, num) {
-  if (!msg && obj instanceof Error) {
-    msg = obj.message
-  }
-  var data = this._baseLog + this._lscache[num] + this.time()
-  // to catch both null and undefined
-  /* eslint-disable eqeqeq */
-  if (msg != undefined) {
-    data += ',"msg":' + JSON.stringify('' + msg)
-  }
-  var value
-  if (obj) {
-    if (obj.stack) {
-      data += ',"type":"Error","stack":' + this.stringify(obj.stack)
+Object.defineProperty(pinoPrototype, 'asJson', {
+  enumerable: true,
+  value: function asJson (obj, msg, num) {
+    if (!msg && obj instanceof Error) {
+      msg = obj.message
     }
-    for (var key in obj) {
-      value = obj[key]
-      if ((!obj.hasOwnProperty || obj.hasOwnProperty(key)) && value !== undefined) {
-        value = this.stringify(this.serializers[key] ? this.serializers[key](value) : value)
-        if (value !== undefined) {
-          data += ',"' + key + '":' + value
+    var data = this._baseLog + this._lscache[num] + this.time()
+    // to catch both null and undefined
+    /* eslint-disable eqeqeq */
+    if (msg != undefined) {
+      data += ',"msg":' + JSON.stringify('' + msg)
+    }
+    var value
+    if (obj) {
+      if (obj.stack) {
+        data += ',"type":"Error","stack":' + this.stringify(obj.stack)
+      }
+      for (var key in obj) {
+        value = obj[key]
+        if ((!obj.hasOwnProperty || obj.hasOwnProperty(key)) && value !== undefined) {
+          value = this.stringify(this.serializers[key] ? this.serializers[key](value) : value)
+          if (value !== undefined) {
+            data += ',"' + key + '":' + value
+          }
         }
       }
     }
+    return data + this.chindings + this.end
   }
-  return data + this.chindings + this.end
-}
+})
 
-Pino.prototype.child = function child (bindings) {
-  if (!bindings) {
-    throw Error('missing bindings for child Pino')
-  }
+Object.defineProperty(pinoPrototype, 'child', {
+  enumerable: true,
+  value: function child (bindings) {
+    if (!bindings) {
+      throw Error('missing bindings for child Pino')
+    }
 
-  var data = ','
-  var value
-  var key
-  for (key in bindings) {
-    value = bindings[key]
-    if (key !== 'level' && key !== 'serializers' && bindings.hasOwnProperty(key) && value !== undefined) {
-      value = this.serializers[key] ? this.serializers[key](value) : value
-      data += '"' + key + '":' + this.stringify(value) + ','
+    var data = ','
+    var value
+    var key
+    for (key in bindings) {
+      value = bindings[key]
+      if (key !== 'level' && key !== 'serializers' && bindings.hasOwnProperty(key) && value !== undefined) {
+        value = this.serializers[key] ? this.serializers[key](value) : value
+        data += '"' + key + '":' + this.stringify(value) + ','
+      }
     }
+    data = this.chindings + data.substr(0, data.length - 1)
+
+    var opts = {
+      level: bindings.level || this.level,
+      levelVal: levels.isStandardLevelVal(this.levelVal) ? undefined : this.levelVal,
+      serializers: bindings.hasOwnProperty('serializers') ? Object.assign({}, this.serializers, bindings.serializers) : this.serializers,
+      stringify: this.stringify,
+      end: this.end,
+      name: this.name,
+      timestamp: this.timestamp,
+      slowtime: this.slowtime,
+      chindings: data,
+      cache: this.cache,
+      formatOpts: this.formatOpts
+    }
+
+    var _child = Object.create(this)
+    _child.stream = this.stream
+    tools.applyOptions.call(_child, opts)
+    return _child
   }
-  data = this.chindings + data.substr(0, data.length - 1)
-
-  var opts = {
-    level: bindings.level || this.level,
-    levelVal: levels.isStandardLevelVal(this.levelVal) ? undefined : this.levelVal,
-    serializers: bindings.hasOwnProperty('serializers') ? Object.assign({}, this.serializers, bindings.serializers) : this.serializers,
-    stringify: this.stringify,
-    end: this.end,
-    name: this.name,
-    timestamp: this.timestamp,
-    slowtime: this.slowtime,
-    chindings: data,
-    cache: this.cache,
-    formatOpts: this.formatOpts
-  }
+})
 
-  var _child = Object.create(this)
-  _child.stream = this.stream
-  tools.applyOptions.call(_child, opts)
-  return _child
-}
+// should this be enumerable?
+Object.defineProperty(pinoPrototype, 'write', {
+  value: function (obj, msg, num) {
+    var s = this.asJson(obj, msg, num)
+    if (!this.cache) {
+      this.stream.write(flatstr(s))
+      return
+    }
 
-Pino.prototype.write = function (obj, msg, num) {
-  var s = this.asJson(obj, msg, num)
-  if (!this.cache) {
-    this.stream.write(flatstr(s))
-    return
+    this.cache.buf += s
+    if (this.cache.buf.length > this.cache.size) {
+      this.stream.write(flatstr(this.cache.buf))
+      this.cache.buf = ''
+    }
   }
+})
+
+Object.defineProperty(pinoPrototype, 'flush', {
+  enumerable: true,
+  value: function () {
+    if (!this.cache) {
+      return
+    }
 
-  this.cache.buf += s
-  if (this.cache.buf.length > this.cache.size) {
     this.stream.write(flatstr(this.cache.buf))
     this.cache.buf = ''
   }
-}
+})
 
-Pino.prototype.flush = function () {
-  if (!this.cache) {
-    return
+Object.defineProperty(pinoPrototype, 'addLevel', {
+  enumerable: true,
+  value: function addLevel (name, lvl) {
+    if (this.levels.values.hasOwnProperty(name)) return false
+    if (this.levels.labels.hasOwnProperty(lvl)) return false
+    this.levels.values[name] = lvl
+    this.levels.labels[lvl] = name
+    this._lscache[lvl] = flatstr('"level":' + Number(lvl))
+    this[name] = tools.genLog(lvl)
+    return true
   }
+})
 
-  this.stream.write(flatstr(this.cache.buf))
-  this.cache.buf = ''
-}
+function pino (opts, stream) {
+  var iopts = opts
+  var istream = stream
+  if (iopts && (iopts.writable || iopts._writableState)) {
+    istream = iopts
+    iopts = defaultOptions
+  }
+  iopts = Object.assign({}, defaultOptions, iopts)
+  if (iopts.extreme && iopts.prettyPrint) throw Error('cannot enable pretty print in extreme mode')
+  istream = istream || process.stdout
+  if (iopts.prettyPrint) {
+    var pstream = pretty(iopts.prettyPrint)
+    var origStream = istream
+    pump(pstream, origStream, function (err) {
+      if (err) instance.emit('error', err)
+    })
+    istream = pstream
+  }
 
-Pino.prototype.addLevel = function addLevel (name, lvl) {
-  if (this.levels.values.hasOwnProperty(name)) return false
-  if (this.levels.labels.hasOwnProperty(lvl)) return false
-  this.levels.values[name] = lvl
-  this.levels.labels[lvl] = name
-  this._lscache[lvl] = flatstr('"level":' + Number(lvl))
-  this[name] = tools.genLog(lvl)
-  return true
+  // internal options
+  iopts.stringify = iopts.safe ? stringifySafe : JSON.stringify
+  iopts.formatOpts = {lowres: true}
+  iopts.end = ',"v":' + LOG_VERSION + '}\n'
+  iopts.cache = !iopts.extreme ? null : {
+    size: 4096,
+    buf: ''
+  }
+  iopts.chindings = ''
+
+  if (iopts.enabled === false) {
+    iopts.level = 'silent'
+  }
+
+  var instance = Object.create(pinoPrototype)
+  instance.stream = istream
+  tools.defineLevelsProperty(instance)
+  tools.applyOptions.call(instance, iopts)
+  if (iopts.cache) setTimeout(waitForFDSettle, 100)
+
+  var settleTries = 0
+  function waitForFDSettle () {
+    var isBlockable = tools.streamIsBlockable(istream)
+    if (isBlockable === false && settleTries > 10) {
+      return instance.emit('error', Error('stream must have a file descriptor in extreme mode'))
+    } else if (isBlockable === true) {
+      return events(instance, extremeModeExitHandler)
+    }
+    settleTries += 1
+    setTimeout(waitForFDSettle, 100)
+  }
+
+  function extremeModeExitHandler () {
+    var buf = iopts.cache.buf
+    if (buf) {
+      // We need to block the process exit long enough to flush the buffer
+      // to the destination stream. We do that by forcing a synchronous
+      // write directly to the stream's file descriptor.
+      var fd = (istream.fd) ? istream.fd : istream._handle.fd
+      fs.writeSync(fd, buf)
+    }
+  }
+
+  return instance
 }
 
+tools.defineLevelsProperty(pino)
+
 module.exports = pino
 module.exports.stdSerializers = {
   req: serializers.asReqValue,
@@ -278,11 +302,3 @@ Object.defineProperty(
   'LOG_VERSION',
   {value: LOG_VERSION, enumerable: true}
 )
-
-// This is an internal API. It can change at any time, including semver-minor.
-// Use it at your own risk.
-Object.defineProperty(
-  module.exports,
-  '_Pino',
-  {value: Pino}
-)


Test patch:
diff --git a/test/levels.test.js b/test/levels.test.js
index 99904f8..6f57b2f 100644
--- a/test/levels.test.js
+++ b/test/levels.test.js
@@ -119,6 +119,40 @@ test('set the level via constructor', function (t) {
   instance.fatal('this is fatal')
 })
 
+test('level-change event', function (t) {
+  var instance = pino()
+  var handle = function (lvl, val, prevLvl, prevVal) {
+    t.is(lvl, 'trace')
+    t.is(val, 10)
+    t.is(prevLvl, 'info')
+    t.is(prevVal, 30)
+  }
+  instance.on('level-change', handle)
+  instance.level = 'trace'
+  instance.removeListener('level-change', handle)
+  instance.level = 'info'
+
+  var count = 0
+
+  var l1 = function () { count += 1 }
+  var l2 = function () { count += 1 }
+  var l3 = function () { count += 1 }
+  instance.on('level-change', l1)
+  instance.on('level-change', l2)
+  instance.on('level-change', l3)
+
+  instance.level = 'trace'
+  instance.removeListener('level-change', l3)
+  instance.level = 'fatal'
+  instance.removeListener('level-change', l1)
+  instance.level = 'debug'
+  instance.removeListener('level-change', l2)
+  instance.level = 'info'
+
+  t.is(count, 6)
+  t.end()
+})
+
 test('enable', function (t) {
   var instance = pino({
     level: 'trace',
@@ -219,37 +253,3 @@ test('setting level in child', function (t) {
   instance.error('this is an error')
   instance.fatal('this is fatal')
 })
-
-test('level-change event', function (t) {
-  var instance = pino()
-  var handle = function (lvl, val, prevLvl, prevVal) {
-    t.is(lvl, 'trace')
-    t.is(val, 10)
-    t.is(prevLvl, 'info')
-    t.is(prevVal, 30)
-  }
-  instance.on('level-change', handle)
-  instance.level = 'trace'
-  instance.removeListener('level-change', handle)
-  instance.level = 'info'
-
-  var count = 0
-
-  var l1 = function () { count += 1 }
-  var l2 = function () { count += 1 }
-  var l3 = function () { count += 1 }
-  instance.on('level-change', l1)
-  instance.on('level-change', l2)
-  instance.on('level-change', l3)
-
-  instance.level = 'trace'
-  instance.removeListener('level-change', l3)
-  instance.level = 'fatal'
-  instance.removeListener('level-change', l1)
-  instance.level = 'debug'
-  instance.removeListener('level-change', l2)
-  instance.level = 'info'
-
-  t.is(count, 6)
-  t.end()
-})