Repository navigation
Benchmark var -> const before merging mass rewrites #8637
Description
Activity
Most of the mass rewrite is in tests. This only really needs to apply to
lib/code right?Reacted by Anna HenningsenProbably. I wouldn't mind the tests running faster either but I doubt it would be noticeable.
Btw, I’m sorry if you feel like this is all a bit much. There were a lot more people showing up than anyone anticipated, so distributing tasks was kind of an ad-hoc thing, and I think most of us see that as a really positive thing (the big turnout) – @Fishrock123 @mscdex @cjihrig @targos and everyone else who’s not busy here and reviewing: you are just the best. ❤️
</offtopic>Reacted by Michaël Zasso, Jeremiah Senkpiel, Johan Bergström, Adri Van Houdt and Gibson FahnestockYes, my primary concern was about code in lib/. I will try to do some benchmarking this weekend and also see what the story is with V8 5.4.
Reacted by Jeremiah Senkpiel and Ron KorvingI've checked
constvsvarvsletin node v4 and v6 with the following script:'use strict' var suite = new require('benchmark').Suite() suite.add('const', function () { const value = 2 var result = 1 for (var i = 0; i < 10000; i++) { result += value } }) suite.add('var', function () { var value = 2 var result = 1 for (var i = 0; i < 10000; i++) { result += value } }) suite.add('let+const', function () { const value = 2 let result = 1 for (let i = 0; i < 10000; i++) { result += value } }) suite.on('cycle', cycle) suite.run() function cycle (e) { console.log(e.target.toString()) }
v6.6.0:
const x 156,967 ops/sec ±0.63% (90 runs sampled) var x 156,124 ops/sec ±0.77% (88 runs sampled) let+const x 34,882 ops/sec ±0.76% (91 runs sampled)v4.5.0:
const x 157,721 ops/sec ±0.80% (86 runs sampled) var x 159,177 ops/sec ±0.74% (91 runs sampled) let+const x 9,881 ops/sec ±0.92% (87 runs sampled)I would say using
constis safe, usingletshould be highly discouraged.I'm not so sure it's quite that easy to benchmark as there could be many factors (e.g. assigning a constant value vs a function call return value, function (de)optimizations, etc.).
Do we have a reason to expect different (better) benchmark result with V8 5.4?
I would suggest the following rule of thumb, until TurboFan becomes the default optimizer (sometime next year):
- Avoid
letfor variables that are modified inside loops. When in doubt, stick withvar.
EDIT: but measure first. In most cases it doesn't matter. Be wary of making generalizations based on micro-benchmarks.
Reacted by Michaël Zasso, James M Snell, Vitor Balocco and Sakthipriyan Vairamani- Avoid
I'd like to point out that by keeping benchmarks using the absolute latest features it makes it impossible to test them against older versions of Node. Requiring editing the file and removing the offending syntax. I'd say for benchmarks that don't involve new JS features we use the backwards compatible syntax as much as possible.
Reacted by Jeremiah Senkpiel, Gibson Fahnestock and AndrasYeah, 100% what @trevnorris said where possible.
- added a commit that references this issue
on Nov 12, 2016 Let/const performance in V8 was recently improved: #9729 (comment)
@nodejs/collaborators
Suggested by @mscdex in #8609 (comment)