events: speed up emit by create faster null prototype object - #39939
events: speed up emit by create faster null prototype object#39939wwwzbwcom wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
@nodejs/v8 is there an explanation to this large difference in speed? |
Sorry, something went wrong.
|
I think Object.create(null) creates the object in "dictionary mode". With this change dictionary mode is not used until some operation determines the transition to it. |
Sorry, something went wrong.
|
The problem with this micro-benchmark is that it tests with only one event name. This is not representative of real use of event emitters. |
Sorry, something went wrong.
|
We need at least to compare the other events benchmarks too. |
Sorry, something went wrong.
|
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1047/ Resultsconfidence improvement accuracy (*) (**) (***) events/ee-add-remove.js n=1000000 removeListener=0 newListener=0 -3.41 % ±4.05% ±5.39% ±7.02% events/ee-add-remove.js n=1000000 removeListener=0 newListener=1 *** 18.79 % ±5.50% ±7.40% ±9.79% events/ee-add-remove.js n=1000000 removeListener=1 newListener=0 ** 5.02 % ±2.99% ±3.99% ±5.23% events/ee-add-remove.js n=1000000 removeListener=1 newListener=1 *** 11.48 % ±2.75% ±3.66% ±4.78% events/ee-emit.js listeners=0 argc=0 n=2000000 *** 316.62 % ±20.23% ±27.20% ±36.00% events/ee-emit.js listeners=0 argc=10 n=2000000 *** 289.75 % ±18.03% ±24.24% ±32.03% events/ee-emit.js listeners=0 argc=2 n=2000000 *** 301.79 % ±11.32% ±15.16% ±19.92% events/ee-emit.js listeners=0 argc=4 n=2000000 *** 290.69 % ±10.78% ±14.46% ±19.06% events/ee-emit.js listeners=10 argc=0 n=2000000 *** 8.23 % ±1.28% ±1.70% ±2.22% events/ee-emit.js listeners=10 argc=10 n=2000000 *** 6.52 % ±1.30% ±1.74% ±2.27% events/ee-emit.js listeners=10 argc=2 n=2000000 *** 8.41 % ±1.32% ±1.76% ±2.29% events/ee-emit.js listeners=10 argc=4 n=2000000 *** 9.18 % ±1.30% ±1.73% ±2.26% events/ee-emit.js listeners=1 argc=0 n=2000000 *** 163.36 % ±11.25% ±15.02% ±19.64% events/ee-emit.js listeners=1 argc=10 n=2000000 *** 163.33 % ±13.79% ±18.51% ±24.42% events/ee-emit.js listeners=1 argc=2 n=2000000 *** 155.27 % ±12.03% ±16.17% ±21.37% events/ee-emit.js listeners=1 argc=4 n=2000000 *** 164.21 % ±9.38% ±12.60% ±16.63% events/ee-emit.js listeners=5 argc=0 n=2000000 *** 18.82 % ±2.40% ±3.20% ±4.17% events/ee-emit.js listeners=5 argc=10 n=2000000 *** 11.39 % ±2.36% ±3.14% ±4.10% events/ee-emit.js listeners=5 argc=2 n=2000000 *** 17.06 % ±2.27% ±3.03% ±3.95% events/ee-emit.js listeners=5 argc=4 n=2000000 *** 18.17 % ±2.53% ±3.38% ±4.43% events/ee-listener-count-on-prototype.js n=50000000 ** -7.75 % ±4.80% ±6.39% ±8.33% events/ee-listeners.js raw='false' listeners=50 n=5000000 -1.54 % ±1.68% ±2.24% ±2.92% events/ee-listeners.js raw='false' listeners=5 n=5000000 *** -3.72 % ±1.69% ±2.25% ±2.92% events/ee-listeners.js raw='true' listeners=50 n=5000000 -1.24 % ±3.07% ±4.12% ±5.44% events/ee-listeners.js raw='true' listeners=5 n=5000000 ** -5.18 % ±3.07% ±4.09% ±5.33% events/ee-once.js argc=0 n=20000000 *** -29.54 % ±0.77% ±1.02% ±1.33% events/ee-once.js argc=1 n=20000000 *** -28.67 % ±1.44% ±1.93% ±2.54% events/ee-once.js argc=4 n=20000000 *** -25.95 % ±1.62% ±2.17% ±2.84% events/ee-once.js argc=5 n=20000000 *** -27.45 % ±0.75% ±1.00% ±1.30% events/eventtarget.js listeners=10 n=1000000 -4.22 % ±5.43% ±7.29% ±9.63% events/eventtarget.js listeners=1 n=1000000 0.13 % ±1.25% ±1.67% ±2.17% events/eventtarget.js listeners=5 n=1000000 -1.53 % ±4.50% ±6.05% ±8.01% |
Sorry, something went wrong.
|
@targos here is the full benchmark result, once event is slower than before. confidence improvement accuracy (*) (**) (***) events/ee-add-remove.js n=1000000 removeListener=0 newListener=0 *** -6.31 % ±1.00% ±1.34% ±1.75% events/ee-add-remove.js n=1000000 removeListener=0 newListener=1 *** 35.24 % ±2.08% ±2.77% ±3.61% events/ee-add-remove.js n=1000000 removeListener=1 newListener=0 *** 19.70 % ±2.03% ±2.71% ±3.53% events/ee-add-remove.js n=1000000 removeListener=1 newListener=1 *** 27.55 % ±1.14% ±1.51% ±1.97% events/ee-emit.js listeners=0 argc=0 n=2000000 *** 273.86 % ±5.62% ±7.57% ±10.05% events/ee-emit.js listeners=0 argc=10 n=2000000 *** 261.12 % ±4.78% ±6.43% ±8.53% events/ee-emit.js listeners=0 argc=2 n=2000000 *** 271.77 % ±4.57% ±6.14% ±8.11% events/ee-emit.js listeners=0 argc=4 n=2000000 *** 272.22 % ±6.62% ±8.91% ±11.80% events/ee-emit.js listeners=1 argc=0 n=2000000 *** 137.57 % ±1.50% ±2.00% ±2.61% events/ee-emit.js listeners=1 argc=10 n=2000000 *** 134.44 % ±4.69% ±6.29% ±8.31% events/ee-emit.js listeners=1 argc=2 n=2000000 *** 130.85 % ±1.92% ±2.58% ±3.41% events/ee-emit.js listeners=1 argc=4 n=2000000 *** 135.05 % ±1.69% ±2.26% ±2.97% events/ee-emit.js listeners=10 argc=0 n=2000000 *** 17.99 % ±1.53% ±2.04% ±2.67% events/ee-emit.js listeners=10 argc=10 n=2000000 *** 14.87 % ±1.12% ±1.50% ±1.96% events/ee-emit.js listeners=10 argc=2 n=2000000 *** 16.37 % ±1.32% ±1.76% ±2.31% events/ee-emit.js listeners=10 argc=4 n=2000000 *** 15.44 % ±1.21% ±1.61% ±2.11% events/ee-emit.js listeners=5 argc=0 n=2000000 *** 19.33 % ±1.06% ±1.41% ±1.84% events/ee-emit.js listeners=5 argc=10 n=2000000 *** 13.76 % ±0.90% ±1.20% ±1.57% events/ee-emit.js listeners=5 argc=2 n=2000000 *** 21.39 % ±2.25% ±3.02% ±4.00% events/ee-emit.js listeners=5 argc=4 n=2000000 *** 14.76 % ±1.60% ±2.14% ±2.82% events/ee-error.js n=100000 -0.06 % ±0.36% ±0.48% ±0.63% events/ee-listener-count-on-prototype.js n=50000000 *** -11.98 % ±1.58% ±2.10% ±2.75% events/ee-listeners.js raw='false' listeners=5 n=5000000 *** -5.10 % ±0.78% ±1.04% ±1.36% events/ee-listeners.js raw='false' listeners=50 n=5000000 -0.87 % ±1.18% ±1.57% ±2.04% events/ee-listeners.js raw='true' listeners=5 n=5000000 *** -6.92 % ±1.34% ±1.78% ±2.32% events/ee-listeners.js raw='true' listeners=50 n=5000000 *** -4.20 % ±1.28% ±1.71% ±2.23% events/ee-once.js argc=0 n=20000000 *** -29.26 % ±0.52% ±0.70% ±0.91% events/ee-once.js argc=1 n=20000000 *** -29.85 % ±1.76% ±2.36% ±3.09% events/ee-once.js argc=4 n=20000000 *** -28.23 % ±0.57% ±0.76% ±0.99% events/ee-once.js argc=5 n=20000000 *** -28.06 % ±0.49% ±0.65% ±0.86% |
Sorry, something went wrong.
benjamingr
left a comment
There was a problem hiding this comment.
This doesn't seem right, I'd assume that Object.create(null) literally calls NewJSObjectWithNullProto which in turn just does NewJSObject (similar to your = {} here) and then SetPrototype.
I'd like to understand this change better first.
Sorry, something went wrong.
|
Looking at the code from what I understand - I'm more confused than when I started :) |
Sorry, something went wrong.
@benjamingr Yes it's strange, here is a simple test: const now = () => {
const hrTime = process.hrtime();
return hrTime[0] * 1000 + hrTime[1] / 1000000;
};
let a, start, end;
a = {};
Object.setPrototypeOf(a, null);
start = now();
for (let i = 0; i < 1e8; i++) {
a['data'];
}
end = now();
console.log(end - start);
a = Object.create(null);
start = now();
for (let i = 0; i < 1e8; i++) {
a['data'];
}
end = now();
console.log(end - start);
/*
64.12246084213257
837.3292689323425
*/
|
Sorry, something went wrong.
@benjamingr I check the source code, looks like Object.create(null) method cant take benefit from fast properties? |
Sorry, something went wrong.
Yeah, but that's the problem of this benchmark. Fast properties work because the object always has the same shape. In the real world, many event emitters are created, with different events, and this is no longer true. |
Sorry, something went wrong.
@targos I dont think so, different ee can have different event, but as some event such store as fast property, we can take benefit when getting its handler |
Sorry, something went wrong.
Also most ee has similar shape such as stream all have data and close events |
Sorry, something went wrong.
|
I don't understand why this would get fast properties in one way or not another - there are several heuristics for fast properties - being a prototype is one but I don't understand why it'd happen here in one case and not the other. (And if it does, I doubt it's a good optimization). To be clear: I am thankful for your contribution and am just confused about the behaviour we're seeing. Can you run the code with v8 native syntax turned on require('v8').setFlagsFromString('--allow-natives-syntax') and then check %HasFastElements? (Or run with --trace-opt --trace-deopt) |
Sorry, something went wrong.
|
Oh and to illustrate the point take the code here:
And see what happens if you invert the order of the test :) (check .create first) |
Sorry, something went wrong.
|
This is definitely a breaking change as there is code in the ecosystem that (rightly or wrongly) accesses _events directly, and switching to a null prototype could definitely break that code update: I forgot we already made the change to a null prototype before... still, out of an abundance of caution here, let's treat this as semver-major until we're definitely sure it's not. If we do decide to make changes here, we should absolutely also test the performance difference of using a Map here also. A long time ago when Map was tested it was definitely slower but I'm not sure if that would still be the case. |
Sorry, something went wrong.
@jasnell Yes, the prototype is alwasy null before and after this change. Map is faster than ObjectCreate(), but Map also has method like toString |
Sorry, something went wrong.
@benjamingr Welcome to Node.js v14.17.3.
Type ".help" for more information.
> require('v8').setFlagsFromString('--allow-natives-syntax')
undefined
> %HasFastProperties(Object.create(null))
false
> %HasFastProperties({})
true
> let a = {};
undefined
> Object.setPrototypeOf(a, null);
[Object: null prototype] {}
> %HasFastProperties(a)
true |
Sorry, something went wrong.
Yes, when you switch the order, the result is different: const now = () => {
const hrTime = process.hrtime();
return hrTime[0] * 1000 + hrTime[1] / 1000000;
};
let a, start, end;
for (let i = 0; i < 10; i++) {
a = Object.create(null);
start = now();
for (let i = 0; i < 1e8; i++) {
a['data'];
}
end = now();
console.log(`Object.create`, end - start);
a = {};
Object.setPrototypeOf(a, null);
start = now();
for (let i = 0; i < 1e8; i++) {
a['data'];
}
end = now();
console.log(`Object.setPrototypeOf`, end - start);
}
/*
Object.create 775.9817728996277
Object.setPrototypeOf 488.72393321990967
Object.create 909.9279327392578
Object.setPrototypeOf 481.3809299468994
Object.create 895.326687335968
Object.setPrototypeOf 473.78826904296875
Object.create 899.6548957824707
Object.setPrototypeOf 474.5387659072876
Object.create 898.921669960022
Object.setPrototypeOf 477.34186267852783
Object.create 895.6246657371521
Object.setPrototypeOf 473.7106132507324
Object.create 894.2418832778931
Object.setPrototypeOf 473.29633617401123
Object.create 908.3455362319946
Object.setPrototypeOf 472.2995858192444
Object.create 895.0272059440613
Object.setPrototypeOf 474.7768030166626
Object.create 894.8622059822083
Object.setPrototypeOf 474.1221857070923
*/ |
Sorry, something went wrong.
|
I would be concerned that in real-world usage the backing object here will deoptimize to dictionary mode anyway. I'd appreciate seeing some more representative benchmarks. |
Sorry, something went wrong.
|
@devsnek I think an argument @wwwzbwcom is making here is that in real-world usage many event emitters only emit 1-2 event types. I see different results for %HasFastProperties after emitting 100 events on each (both are fast properties :)) |
Sorry, something went wrong.
Agree that, some realworld usage benchmark is need. |
Sorry, something went wrong.
|
@devsnek slightly improvement in readable-from, more realworld benchmark later node benchmark/compare.js --old ./node-master --new ./node-dev --filter "readable-from" streams > compare.csv cat compare.csv | Rscript benchmark/compare.R confidence improvement accuracy (*) (**) (***) streams/readable-from.js n=10000000 ** 17.47 % ±11.10% ±14.78% ±19.26% |
Sorry, something went wrong.
|
I'll explain why I'm suspicious. 17.8% is a big improvement in streams/readable-from.js and I don't understand why/how this change explains it. |
Sorry, something went wrong.
|
Late to the party. You're observation is correct, both {__proto__:null} and Object.create(null) will create dictionary/slow-mode objects in V8 (and if you're really performance sensitive, use the literal syntax as it avoids additional property loads and a method call). This is based on the assumptions that folks use null-proto objects as dictionaries, in which case the fast-mode structure-tracking in V8 causes more harm than good. Especially all secondary effects with ICs are not represented in your micro benchmark, which is also one of the main motivations for "exotic" behavior in V8. The reasoning behind this, is that if you have known property-names you're likely fine with non-null prototype, since you don't accidentally read those. However, with unknown property names (and that's the dictionary/Map use-case), you have to use a null-proto in order to get correct behavior. Thus the null-proto is quite a good indication for this behavior. Let me know if you I can help with additional information. |
Sorry, something went wrong.
|
@camillobruni thanks that's educational! and setting const o = {}; Object.setPrototypeOf(o, null); does not have the same optimisation of assuming it's used as a dictionary? |
Sorry, something went wrong.
|
I think that's quite an uncommon case that's not widely adopted for this pattern. Would a custom non-null prototype als be a solution for you? const EventProto = {__proto__:null};
...
this._events = {__proto__: EventProto};
...
|
Sorry, something went wrong.
|
I think it makes sense to default to shapes/hidden-classes here since it'll fall back to "slow mode" anyway if you add many handlers. Note that event lists aren't really dictionaries since often you sign up to multiple events once and then you dispatch events many times. That said, I have no idea why the Readable.from benchmark changed 17%, Readable.from doesn't even subscribe to any events it just reads from an async iterator. |
Sorry, something went wrong.
in this benchmark, it does not just read from the stream, but create a handler on the stream "data" event, so as the ee is faster, this benchmark change 17% |
Sorry, something went wrong.
Sorry, something went wrong.
|
@ronag I meant - what do you think of this change (optimizing for subscribing to fewer static events treating event lists in emitters as more static than not - from the streams PoV) |
Sorry, something went wrong.
From stream point of view I think it's a good idea. Streams usually don't add/remove events and there is a limited number of events (data/drain/readable, end/finish, error, close). I think we've tried avoid dynamic patterns. So usually there is 4 static events + handlers per stream. |
Sorry, something went wrong.
|
Ok, I think I'm fine with this :) It:
Also ping @mscdex |
Sorry, something went wrong.
|
I am going to close this due to the issues discussed and since we didn't get a conclusion until now. Please open a new PR, if you believe we should still do this. |
Sorry, something went wrong.
We need to create null prototype to prevent internal method like toString mess with event handling.
But in current version v8, Object create by ObjectSetPrototypeOf is much more efficient than ObjectCreate(null)
Here is benchmark result: