benchmark: add microbenchmarks for ES Map #7581

rvagg · 2016-07-07T11:03:14Z

Given the discussion in #6102 about Map still being too slow to use I figured I'd see with a benchmark. @mscdex, @jasnell please review and let me know if you think this is valid.

es/map-bench.js method="pojo" millions="10": 1.07989
es/map-bench.js method="fakeMap" millions="10": 0.85223
es/map-bench.js method="map" millions="10": 1.77412

As for why the fakeMap test is faster ... I got nothing ...

Update after @fhinkel noticed a serious flaw:

v6

es/map-bench.js millions=1 method="object": 0.18984850416666324
es/map-bench.js millions=1 method="nullProtoObject": 0.16044478686078825
es/map-bench.js millions=1 method="fakeMap": 0.1311882248350222
es/map-bench.js millions=1 method="map": 0.3121014937928409

master

es/map-bench.js millions=1 method="object": 0.2220851555085364
es/map-bench.js millions=1 method="nullProtoObject": 0.19131381289232038
es/map-bench.js millions=1 method="fakeMap": 0.17931948397493905
es/map-bench.js millions=1 method="map": 0.28127750318241235

Times are ops/sec, so higher is better

Fishrock123 · 2016-07-07T11:04:35Z

benchmark/es/map-bench.js

+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  method: ['pojo', 'fakeMap', 'map'],


plain old javascript object ... comes from plain old java object, maybe that's just for us old java dogs

I'll just call it 'object' since it's not obvious

addaleax · 2016-07-07T11:07:04Z

Not sure if it matters, but it might be nice to have one for m = Object.create(null), too.

rvagg · 2016-07-07T11:14:56Z

ok, s/pojo/object and added one for a null-prototype object

es/map-bench.js method="object" millions="10": 1.07722
es/map-bench.js method="nullProtoObject" millions="10": 1.04962
es/map-bench.js method="fakeMap" millions="10": 0.88726
es/map-bench.js method="map" millions="10": 1.78555

cjihrig · 2016-07-07T13:34:02Z

Should this utilize common.v8ForceOptimization()?

cjihrig · 2016-07-07T13:35:10Z

benchmark/es/map-bench.js

+
+function runObject(n) {
+  const m = {};
+  let i = 0;


I think we're still preferring var over let.

rvagg · 2016-07-07T13:42:36Z

s/let/var, thanks for pointing that out.

I don't know if this is a candidate for common.v8ForceOptimization() or not, there's a bit of state to pass around, it'd at least be passing in m to optimized functions from the parent bench functions, would that make a difference?

cjihrig · 2016-07-07T13:46:13Z

I'll defer to someone with more experience working on the benchmarks, but es/restparams-bench.js, for example, forces optimization with arguments.

mscdex · 2016-07-07T15:08:44Z

AFAIK that should be (million) ops/sec, so technically Map would be the fastest (largest value). That's quite surprising.

jasnell · 2016-07-07T15:57:58Z

Interesting! Map is definitely showing to be faster... even if you take creation of the Map into consideration on each loop.

Unmodified version:

~/node/main/node [check] $ ./node benchmark/es/map-bench.js
es/map-bench.js method="object" millions="10": 0.84279
es/map-bench.js method="nullProtoObject" millions="10": 0.84733
es/map-bench.js method="fakeMap" millions="10": 0.69652
es/map-bench.js method="map" millions="10": 1.71819

Modified to include map creation on each loop:

~/node/main/node [check] $ ./node benchmark/es/map-bench.js
es/map-bench.js method="object" millions="10": 0.77879
es/map-bench.js method="nullProtoObject" millions="10": 0.77240
es/map-bench.js method="fakeMap" millions="10": 0.37927
es/map-bench.js method="map" millions="10": 1.61652

rvagg · 2016-07-08T06:40:38Z

ok then, that is interesting and explains why fakeMap looked "better" than the pojo, perhaps we go ahead and migrate headers to a Map eh?

mscdex · 2016-07-08T07:14:38Z

@rvagg That would probably have to happen at least in v7 or later since that would be a pretty big change.

simonkcleung · 2016-07-10T07:02:44Z

The number should be the higher the better?

mscdex · 2016-07-10T07:41:06Z

@simonkcleung Yes. Values are always operations per second.

jasnell · 2016-07-20T15:57:22Z

Moving the querystring and headers objects to Maps definitely makes sense given the perf results but @Fishrock123 is right about targeting such a change for v7. We can likely go ahead and get that change landed in master now as a semver-major.

fhinkel · 2016-07-29T06:44:07Z

benchmark/es/map-bench.js

+  var i = 0;
+  bench.start();
+  for (; i < n; i++) {
+    m['i' + n] = n;


Are we trying to set the same two properties ('i10000000' and 's10000000') 10 million times? If so, maybe use keys like 'a', 'b'. Or should the keys be 'i' + i and 's' + i?

fhinkel · 2016-07-29T07:53:16Z

@rvagg Thanks for putting this together.

When we're using named keys o[key] on an object, access is usually megamorphic. V8 only has limited space in the megamorphic IC stub cache, around 2500 entries. The same cache is used for all megamorphic ICs, which can be completely unrelated to o.

Every time we add a property to an object, we create a new transition map. Only if we add properties in exactly the same order can we reuse the maps. This is often not the case, for example when we parse request headers into _headers[key]. Because we need to keep track of the transitions, the number of maps quickly increases. All these maps fight for entries in the megamorphic IC stub cache, eventually overwriting maps for other objects that are unrelated to o, which slows down the application overall.

So +1 for moving to maps!

jasnell · 2016-07-29T15:56:15Z

btw, LGTM for getting this landed. :-)

jasnell · 2016-08-04T20:48:35Z

@rvagg ... is this ready to land?

addaleax · 2016-08-04T21:05:04Z

I think @fhinkel’s comment should still be addressed?

addaleax · 2016-09-20T14:18:09Z

ping @rvagg

rvagg · 2016-10-18T03:11:44Z

Thanks for catching that @fhinkel, that changes everything!

Updates pushed, pls review folks.

v6

es/map-bench.js thousands=100 method="object": 0.26339184977485885
es/map-bench.js thousands=100 method="nullProtoObject": 0.23501547831916525
es/map-bench.js thousands=100 method="fakeMap": 0.212913507413461
es/map-bench.js thousands=100 method="map": 0.7449254838002358

master

es/map-bench.js thousands=100 method="object": 0.35728933045594424
es/map-bench.js thousands=100 method="nullProtoObject": 0.029615543283847124
es/map-bench.js thousands=100 method="fakeMap": 0.028427891784159526
es/map-bench.js thousands=100 method="map": 0.15582199860352638

fhinkel · 2016-10-18T08:54:01Z

LGTM. Should we update the first comment with the new times and also point out, that it's ops/sec rather than seconds (so higher is faster).

rvagg · 2016-10-18T09:37:27Z

Good idea, done! @fhinkel is there a known speed regression for maps in later V8? We're comparing 5.1 to 5.4 here and that's a big slowdown, or could the test not be getting at the real numbers?

fhinkel · 2016-10-18T09:50:08Z

I wonder if something else was going on during the benchmarking, because nullProtoObject and fakeMap also have a 10x slowdown. Let me try on my machine ...

rvagg · 2016-10-18T09:59:36Z

yea, I think you're right, new run on master:

es/map-bench.js thousands=100 method="object": 0.10768888702936505
es/map-bench.js thousands=100 method="nullProtoObject": 0.11350308225508328
es/map-bench.js thousands=100 method="fakeMap": 0.12039689515526422
es/map-bench.js thousands=100 method="map": 0.38445207528844944

rvagg · 2016-10-18T10:03:49Z

Too much variability between runs, bumping it back up to the millions and it stabilises a bit more:

v6

es/map-bench.js millions=1 method="object": 0.18984850416666324
es/map-bench.js millions=1 method="nullProtoObject": 0.16044478686078825
es/map-bench.js millions=1 method="fakeMap": 0.1311882248350222
es/map-bench.js millions=1 method="map": 0.3121014937928409

master

es/map-bench.js millions=1 method="object": 0.2220851555085364
es/map-bench.js millions=1 method="nullProtoObject": 0.19131381289232038
es/map-bench.js millions=1 method="fakeMap": 0.17931948397493905
es/map-bench.js millions=1 method="map": 0.28127750318241235

fhinkel · 2016-10-18T10:28:15Z

I think we can blame the variation on timing issues. I don't get a regression going from v6.0.0 to master.

Node v6.0.0:

~/node/benchmark/es$ node --version
v6.0.0
~/node/benchmark/es$ node -e "console.log(process.versions.v8)"
5.0.71.35
~/node/benchmark/es$ node map-bench.js 
es/map-bench.js millions=1 method="object": 0.3697918357000556
es/map-bench.js millions=1 method="nullProtoObject": 0.41595417356334285
es/map-bench.js millions=1 method="fakeMap": 0.36001723720769746
es/map-bench.js millions=1 method="map":  0.5557669893876821

Node v6.7.0:

~/node/benchmark/es$ node -e "console.log(process.versions.v8)"
5.1.281.83
~/node/benchmark/es$ node map-bench.js 
es/map-bench.js millions=1 method="object": 0.4192028932525165
es/map-bench.js millions=1 method="nullProtoObject": 0.43625871109888
es/map-bench.js millions=1 method="fakeMap": 0.3631661863732719
es/map-bench.js millions=1 method="map": 0.5750817453541195

Node master:

~/node/benchmark/es$ ~/node/node -e "console.log(process.versions.v8)"
5.5.0 (candidate)
~/node/benchmark/es$ ~/node/node map-bench.js 
es/map-bench.js millions=1 method="object": 0.43575744375713865
es/map-bench.js millions=1 method="nullProtoObject": 0.4588149868110948
es/map-bench.js millions=1 method="fakeMap": 0.4051635672142672
es/map-bench.js millions=1 method="map": 0.6342556913214041

rvagg · 2016-10-29T07:05:12Z

landed @ 07cc9df

PR-URL: #7581 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>

nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Jul 7, 2016

Fishrock123 reviewed Jul 7, 2016
View reviewed changes

rvagg force-pushed the es-map-bench branch from 4da7abe to 77ef0ed Compare July 7, 2016 11:12

cjihrig reviewed Jul 7, 2016
View reviewed changes

benchmark/es/map-bench.js

function runObject(n) {

const m = {};

let i = 0;

Copy link

Contributor

cjihrig Jul 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're still preferring var over let.

rvagg force-pushed the es-map-bench branch from 77ef0ed to b343185 Compare July 7, 2016 13:43

mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Jul 7, 2016

ofrobots mentioned this pull request Jul 22, 2016

http: don't inherit from Object.prototype #6102

Closed

3 tasks

fhinkel reviewed Jul 29, 2016
View reviewed changes

Florian-R mentioned this pull request Sep 20, 2016

timers: improve setTimeout/Interval performance #8661

Merged

2 tasks

addaleax mentioned this pull request Sep 20, 2016

module: NativeModule._cache turned to a Map #8576

Closed

2 tasks

Trott force-pushed the master branch from b0df363 to c5ce7f4 Compare September 21, 2016 00:09

rvagg added 2 commits October 18, 2016 13:56

benchmark: add microbenchmarks for ES Map

8b75639

squash

15bd927

rvagg force-pushed the es-map-bench branch from b343185 to 15bd927 Compare October 18, 2016 03:11

squash, back up to 1 million

b550a78

rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02

rvagg closed this Oct 29, 2016

rvagg deleted the es-map-bench branch October 29, 2016 07:05

rvagg added a commit that referenced this pull request Oct 29, 2016

benchmark: add microbenchmarks for ES Map

07cc9df

PR-URL: #7581 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>

evanlucas pushed a commit that referenced this pull request Nov 3, 2016

benchmark: add microbenchmarks for ES Map

fab8eb6

PR-URL: #7581 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>

evanlucas mentioned this pull request Nov 3, 2016

v7.1.0 proposal - 2016-11-08 #9438

Merged

MylesBorins pushed a commit that referenced this pull request Nov 22, 2016

benchmark: add microbenchmarks for ES Map

5eeac8c

PR-URL: #7581 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>

MylesBorins added land-on-v4.x labels Nov 22, 2016

MylesBorins pushed a commit that referenced this pull request Nov 22, 2016

benchmark: add microbenchmarks for ES Map

ed31f9c

PR-URL: #7581 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>

This was referenced Nov 22, 2016

v6.9.2 proposal #9735

Merged

v4.7.0 proposal #9736

Merged

lpinca mentioned this pull request Jan 28, 2017

timers: use Maps for internal duration pooling #11040

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

benchmark: add microbenchmarks for ES Map #7581

benchmark: add microbenchmarks for ES Map #7581

rvagg commented Jul 7, 2016 •

edited

Loading

Fishrock123 Jul 7, 2016

rvagg Jul 7, 2016

rvagg Jul 7, 2016

addaleax commented Jul 7, 2016

rvagg commented Jul 7, 2016

cjihrig commented Jul 7, 2016

cjihrig Jul 7, 2016

rvagg commented Jul 7, 2016

cjihrig commented Jul 7, 2016

mscdex commented Jul 7, 2016

jasnell commented Jul 7, 2016

rvagg commented Jul 8, 2016

mscdex commented Jul 8, 2016

simonkcleung commented Jul 10, 2016

mscdex commented Jul 10, 2016

jasnell commented Jul 20, 2016

fhinkel Jul 29, 2016

fhinkel commented Jul 29, 2016

jasnell commented Jul 29, 2016

jasnell commented Aug 4, 2016

addaleax commented Aug 4, 2016

addaleax commented Sep 20, 2016

rvagg commented Oct 18, 2016

fhinkel commented Oct 18, 2016

rvagg commented Oct 18, 2016

fhinkel commented Oct 18, 2016

rvagg commented Oct 18, 2016

rvagg commented Oct 18, 2016

fhinkel commented Oct 18, 2016 •

edited

Loading

rvagg commented Oct 29, 2016

benchmark: add microbenchmarks for ES Map #7581

benchmark: add microbenchmarks for ES Map #7581

Conversation

rvagg commented Jul 7, 2016 • edited Loading

Fishrock123 Jul 7, 2016

Choose a reason for hiding this comment

rvagg Jul 7, 2016

Choose a reason for hiding this comment

rvagg Jul 7, 2016

Choose a reason for hiding this comment

addaleax commented Jul 7, 2016

rvagg commented Jul 7, 2016

cjihrig commented Jul 7, 2016

cjihrig Jul 7, 2016

Choose a reason for hiding this comment

rvagg commented Jul 7, 2016

cjihrig commented Jul 7, 2016

mscdex commented Jul 7, 2016

jasnell commented Jul 7, 2016

rvagg commented Jul 8, 2016

mscdex commented Jul 8, 2016

simonkcleung commented Jul 10, 2016

mscdex commented Jul 10, 2016

jasnell commented Jul 20, 2016

fhinkel Jul 29, 2016

Choose a reason for hiding this comment

fhinkel commented Jul 29, 2016

jasnell commented Jul 29, 2016

jasnell commented Aug 4, 2016

addaleax commented Aug 4, 2016

addaleax commented Sep 20, 2016

rvagg commented Oct 18, 2016

fhinkel commented Oct 18, 2016

rvagg commented Oct 18, 2016

fhinkel commented Oct 18, 2016

rvagg commented Oct 18, 2016

rvagg commented Oct 18, 2016

fhinkel commented Oct 18, 2016 • edited Loading

rvagg commented Oct 29, 2016

rvagg commented Jul 7, 2016 •

edited

Loading

fhinkel commented Oct 18, 2016 •

edited

Loading