events: handle inherited properties properly

[thefourtheye]

As of now, the events module considers inherited properties as one of the
valid types, by default.

> process.version
'v3.0.0'
> events.EventEmitter.listenerCount(new events.EventEmitter(), 'toString')
1

This patch makes sure that the inherited properties are considered as
normal types and they will not be counted unless explicitly added.

> process.version
'v4.0.0-pre'
> events.EventEmitter.listenerCount(new events.EventEmitter(), 'toString')
0
> const emitter = new events.EventEmitter();
undefined
> emitter.on('toString', function() {});
EventEmitter {
domain:
Domain {
domain: null,
_events: { error: [Function] },
_eventsCount: 1,
_maxListeners: undefined,
members: [] },
_events: { toString: [Function] },
_eventsCount: 1,
_maxListeners: undefined }
> events.EventEmitter.listenerCount(emitter, 'toString')
1

I am guessing this would be a semver-major change, as this could be a breaking change.

[vkurchatkin]

nodejs/node-v0.x-archive#4366

[thefourtheye]

Oh. Performance hit. Okay, how about using a Map instead?

[vkurchatkin]

#1785

[brendanashworth]

Could someone run benchmarks on the entire new EventEmitter()?

The event emitter code is considered a red hot code path and obj = Object.create(null) is 20-30 times slower than obj = {}.

This doesn't seem to be benchmarking much, obj = {} could be a thousandth of the actual time spent creating the EE.

[brendanashworth]

Is this a necessary change?

[thefourtheye]

Closing in favour of #1785

[thefourtheye]

Reopening this thread as #1785 was shot down and Object.create is ten times slower than Object literal but the difference is a very small number. The script I used is at #1785 (comment) cc @nodejs/collaborators

[mscdex]

@thefourtheye I'm not seeing the same results that you described in that other issue. Here is what I get with current master and your benchmark script:

Object.create: 7359.860ms
Object Literal: 695.927ms

[targos]

I'm seeing the same kind of benchmark results but I'm not sure we can trust them. V8 may be smart enough to see that we don't do anything with the object literal and not create it at all.

[targos]

@thefourtheye can you compare the EE benchmarks between master and master + this PR ?

[mscdex]

@targos I just added code that interacts with obj in both scenarios and I still see the same results.

[jasnell]

@thefourtheye ... is this still something you want to pursue?

[thefourtheye]

@jasnell Yes, as this has to be fixed somehow. I'll close this now and open a separate PR.