outmoded / hapi-contrib

Discussion forum for project contributors
Other
78 stars 25 forks source link

Update for loop directions #107

Closed Lukeghenco closed 7 years ago

Lukeghenco commented 7 years ago

It is more performant to declare the length of a variable before a for loop. Array.prototype.length is not a cheap action.

Marsup commented 7 years ago

It is a cheap action. Many benchmarks proved this point over and over.

skeggse commented 7 years ago

See also discussion on #59

Lukeghenco commented 7 years ago

Thanks @Marsup and @skeggse for your comments. Inline caching of a for loop is negligible, but declaring it as a variable before hand can be marginally faster depending on the web browser or V8 you are using. If you would like to take a look at the Array.prototype.forEach2(). This has been similar to the results that I have gotten on performance testing. I would never go as far as to say it is the wrong way to do it. That was my intent of this contribution.

Marsup commented 7 years ago

We do say this because most modern browsers (and even old ones) know perfectly how to optimize this and don't need your help with caching the length. I see absolutely no evidence that would encourage us to go back to this micro-optimization.

DavidTPate commented 7 years ago

These types of potential optimizations I was curious about a few months ago, I put together a simple set of benchmarks for various types of look that you could try to run @Lukeghenco to see for yourself over at DavidTPate/js-bench.

In my tests the non-variable loops (first code block below) has consistently out-performed (very slightly) the loop which stores the length in a variable (second code block below).

Length looked up:

for (var i = 0; i < arrayToLoop.length; i++) {
  // Do something
}

Length stored in a variable:

for (var i = 0, il = arrayToLoop.length; i < il; i++) {
  // Do something
}

Here were my results from a recent test on Node v4.7.3 (the ones with static in the name are utilizing a variable, while the others are not).

loops#for-check-length              x 3,836,410 ops/sec ±1.44% (88 runs sampled)
loops#for-length-variable           x 3,784,995 ops/sec ±1.71% (89 runs sampled)
loops#for-static-length             x 3,633,953 ops/sec ±1.50% (85 runs sampled)
loops#for-static-length-reverse     x 3,770,956 ops/sec ±1.53% (89 runs sampled)
Lukeghenco commented 7 years ago

Thanks @DavidTPate that is a really nice performance test. I ran it a few times and with Node 6.9.5 it seemed to run the same performance for me too. It must just be the jsPerf that is giving some fake performance results. I appreciate the help @Marsup @skeggse and @DavidTPate.

DavidTPate commented 7 years ago

@Lukeghenco No problem!

So keep in mind that JSPerf is operating within a different environment from Node. It's not that it is providing you with incorrect results they are just results for a different environment.

You can find the same tests I previously referenced also on jsperf and there's a significant difference in performance results (though still similar results as to what is more performant), again it's a different environment.