benchmark: add microbenchmarks for ES Map by rvagg · Pull Request #7581 · nodejs/node · GitHub

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

pojo?

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

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

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

Should this utilize common.v8ForceOptimization()?

I think we're still preferring var over let.

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?

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

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

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

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?

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

The number should be the higher the better?

@simonkcleung Yes. Values are always operations per second.

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.

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?

@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!

btw, LGTM for getting this landed. :-)

@rvagg ... is this ready to land?

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

ping @rvagg

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

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).

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?

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 ...

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

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

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

landed @ 07cc9df

PR-URL: #7581 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <ranziska.hinkelmann@gmail.com>

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

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 <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <ranziska.hinkelmann@gmail.com>

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 <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <ranziska.hinkelmann@gmail.com>

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 <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <ranziska.hinkelmann@gmail.com>

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 <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <ranziska.hinkelmann@gmail.com>

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

benchmark: add microbenchmarks for ES Map by rvagg · Pull Request #7581 · nodejs/node · GitHub

Uh oh!

Uh oh!

benchmark: add microbenchmarks for ES Map - #7581

benchmark: add microbenchmarks for ES Map#7581rvagg wants to merge 3 commits intonodejs:masternodejs/node:masterfrom rvagg:es-map-benchrvagg/io.js:es-map-benchCopy head branch name to clipboard

Conversation

rvagg commented Jul 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fishrock123 Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

rvagg Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

rvagg Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

addaleax commented Jul 7, 2016

Uh oh!

rvagg commented Jul 7, 2016

Uh oh!

cjihrig commented Jul 7, 2016

Uh oh!

cjihrig Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

rvagg commented Jul 7, 2016

Uh oh!

cjihrig commented Jul 7, 2016

Uh oh!

mscdex commented Jul 7, 2016

Uh oh!

jasnell commented Jul 7, 2016

Uh oh!

rvagg commented Jul 8, 2016

Uh oh!

mscdex commented Jul 8, 2016

Uh oh!

simonkcleung commented Jul 10, 2016

Uh oh!

mscdex commented Jul 10, 2016

Uh oh!

jasnell commented Jul 20, 2016

Uh oh!

fhinkel Jul 29, 2016

Choose a reason for hiding this comment

Uh oh!

fhinkel commented Jul 29, 2016

Uh oh!

jasnell commented Jul 29, 2016

Uh oh!

jasnell commented Aug 4, 2016

Uh oh!

addaleax commented Aug 4, 2016

Uh oh!

addaleax commented Sep 20, 2016

Uh oh!

rvagg commented Oct 18, 2016

Uh oh!

fhinkel commented Oct 18, 2016

Uh oh!

rvagg commented Oct 18, 2016

Uh oh!

fhinkel commented Oct 18, 2016

Uh oh!

rvagg commented Oct 18, 2016

Uh oh!

rvagg commented Oct 18, 2016

Uh oh!

fhinkel commented Oct 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rvagg commented Oct 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

benchmark: add microbenchmarks for ES Map#7581
rvagg wants to merge 3 commits into
nodejs:masterfrom
rvagg:es-map-bench

rvagg commented Jul 7, 2016 •
edited
Loading

fhinkel commented Oct 18, 2016 •
edited
Loading