v8: add benchmarks to serialize/deserialize and improve if statements #43937
Conversation
anonrig
force-pushed
the
perf/v8-deserialize-serialize
branch
from
aa6fd55 to
9579939
Compare
anonrig
changed the title
lib/v8.js
Outdated
| if (index === 11) return BigInt64Array; | ||
| if (index === 12) return BigUint64Array; | ||
| return undefined; | ||
| switch (index) { |
There was a problem hiding this comment.
What about using an array-based lookup table here? That would probably be even more efficient, especially for the last cases.
There was a problem hiding this comment.
It's faster, I'm updating my changes
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-with-array ./benchmark/v8/serialize.js
v8/serialize.js n=1000000: 637,198.2984177626
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-perf-v8 ./benchmark/v8/serialize.js
v8/serialize.js n=1000000: 625,952.948029536
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-with-array ./benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,499,926.0363972564
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-perf-v8 ./benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,435,339.223199034
| if (type === '[object BigUint64Array]') return 12; | ||
| return -1; | ||
|
|
||
| switch (type) { |
There was a problem hiding this comment.
How does this compare to an object or Map lookup?
There was a problem hiding this comment.
Object is slower compared to switch:
With switch:
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-debug benchmark/v8/serialize.js
v8/serialize.js n=1000000: 705,193.2134097881
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-debug benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,595,554.4661464228
With object:
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-v8-object benchmark/v8/serialize.js
v8/serialize.js n=1000000: 696,654.8116336825
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-v8-object benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,575,265.6004526608
There was a problem hiding this comment.
Map is faster on serialize but slower on deserialize:
With switch:
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-master benchmark/v8/serialize.js
v8/serialize.js n=1000000: 690,649.3698025825
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-master benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,582,109.8185933107
With Map:
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-v8-map benchmark/v8/serialize.js
v8/serialize.js n=1000000: 695,808.1758001995
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-v8-map benchmark/v8/serialize.js
v8/serialize.js n=1000000: 699,940.6800273677
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-v8-map benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,565,646.8890889874
➜ node git:(perf/v8-deserialize-serialize) ✗ ./node-v8-map benchmark/v8/deserialize.js
v8/deserialize.js n=1000000: 1,572,391.3241736297
There was a problem hiding this comment.
That doesn't make sense to me since the current serialize benchmark included in this PR doesn't even exercise this code path and for deserialize it'd never hit this code path because it's for serialization only.
There was a problem hiding this comment.
It should be noted when running benchmarks you can't just run them once and compare because the results can vary. So you either need to run them manually a bunch of times to get a feel for the expected range or use the compare.js script to do all of this for you and tell you whether results are statistically significant.
There was a problem hiding this comment.
I changed the serialize benchmark to this, which does exercise the code path in question:
'use strict';
const common = require('../common.js');
const v8 = require('v8');
const bench = common.createBenchmark(main, {
n: [1e6]
});
function main({ n }) {
const abv = new BigUint64Array();
bench.start();
for (let i = 0; i < n; i++)
v8.serialize({ a: 1, b: abv });
bench.end(n);
}Using that benchmark against a plain object, a plain null object, and a Map, all of them have either the same or less performance compared to the if/switch solution.
anonrig
force-pushed
the
perf/v8-deserialize-serialize
branch
from
9579939 to
74abb29
Compare
anonrig
force-pushed
the
perf/v8-deserialize-serialize
branch
from
74abb29 to
33b9ce7
Compare
| }); | ||
|
|
||
| function main({ n }) { | ||
| const serialized = v8.serialize({ a: 1 }); |
There was a problem hiding this comment.
| const serialized = v8.serialize({ a: 1 }); | |
| const serialized = v8.serialize({ a: 1, b: new BigUint64Array() }); |
| bench.start(); | ||
| for (let i = 0; i < n; i++) | ||
| v8.serialize({ a: 1 }); |
There was a problem hiding this comment.
| bench.start(); | |
| for (let i = 0; i < n; i++) | |
| v8.serialize({ a: 1 }); | |
| const typedArray = new BigUint64Array(); | |
| bench.start(); | |
| for (let i = 0; i < n; i++) | |
| v8.serialize({ a: 1, b: typedArray }); |
3 participants
I added 2 benchmarks to serialize and deserialize method in order to understand the performance impact of replacing a really big if statement with a switch statement. Here are my findings:
Serialize
Before
After
Deserialize
Before
After