Prossible perf_hooks memory leak detected

[Jokcy]

It seems nanobus creates a performance entry on event emit and never clear the entry, so after a long time of using, more and more entries of performance will be in memory.

key code here:

var emitTiming = nanotiming(this._name + "('" + eventName + "')")
  var listeners = this._listeners[eventName]
  if (listeners && listeners.length > 0) {
    this._emit(this._listeners[eventName], data)
  }

  if (this._starListeners.length > 0) {
    this._emit(this._starListeners, eventName, data, emitTiming.uuid)
  }
  emitTiming()

and warning message is:

(node:84217) Warning: Possible perf_hooks memory leak detected. There are 996 entries in the Performance Timeline. Use the clear methods to remove entries that are no longer needed or set performance.maxEntries equal to a higher value (currently the maxEntries is 150).

and my test code is here:

const nanobus = require('nanobus')
const bus = nanobus()

bus.on('foo', () => {})

for (let i=0; i<1000; i++) {
  bus.emit('foo', 'bus')
}

I see this message when I use webpack-serve which use nanobus as bus to emit events.

So I guess it's a bug?

[yoshuawuyts]

The quickest fix for this is to disable performance timings all together: https://github.com/choojs/nanotiming#disabling-timings.

But need to dig a bit deeper into this to fix the root cause.

[yoshuawuyts]

I believe this is related: nodejs/node#19563

[yoshuawuyts]

While perf_hooks are still experimental, remove the performance.getEntries() and performance.clear*() variants and eliminate the accumulation of the global timeline entries. (...)

The patch above was landed to prevent this exact scenario from occurring. @Jokcy what version of Node are you using? Looks like >=10.0.0 should have this change.

[Jokcy]

@yoshuawuyts Oh my node version is 9.11.1. so what I need to do is just upgrade my node or set DISABLE_NANOTIMING env ?

[yoshuawuyts]

@Jokcy yep, that sounds about right. I was hoping the new timing API would have been backported to 9.x.x but unfortunately it hasn't been. On the other hand: 9.x is only being supported until the end of the month so maybe it's okay to not backport feature detection / support?

[yoshuawuyts]

Oh yeah, thought of another tweak we could do: we could detect the node version and switch off perf timings if the version is <10. Makes it so there's less compat code to worry about :D