Skip to content

store arr.length in a variable in for-loop #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

meowwwls
Copy link

@meowwwls meowwwls commented Oct 21, 2016

When something will be checked multiple times (such as the array length in a loop) and the value will not change, it's best to store that value in a variable.

I've edited the suggested for-loop construct to include storing the array length in a variable and checking the current index against that variable, to prevent the array length from being evaluated on each iteration.

@GabrieleRomeo
Copy link
Contributor

Hi guys, from my personal point of view checking for the array.length inside the for statement is a bad choice because it requires the evaluation of the same thing over and over again even though it always remains the same thing

@meowwwls
Copy link
Author

meowwwls commented Oct 21, 2016

@GabrieleRomeo If the length is stored in a variable (just like we store the index in a variable), the length is not evaluated each time the loop runs. The length is only evaluated at the time of the variable creation.

var l = arr.length;

for (var i = 0; i < l; i++) {
  // do stuff
}

is no different than

for (var i = 0, l = arr.length; i < l; i++) {
  // do stuff
}

The only way the array length is evaluated every time the loops runs is when the code like this:

for (var i = 0; i < arr.length; i++) {
  // do stuff
}

which is what was originally in the style guide. I suggested the edit or storing the array length in a variable to prevent this.

@GabrieleRomeo
Copy link
Contributor

@meowwwls you're right!

I was referring to this:

for (var i = 0; i < arr.length; i++) {
    console.log(arr[i]);
    // ...
}

Your version is perfect 😉

@meowwwls
Copy link
Author

@GabrieleRomeo

Sorry about that! I misunderstood your original comment. :)

@jllodra
Copy link
Contributor

jllodra commented Oct 27, 2016

Hi @meowwwls and @GabrieleRomeo,

First of all, thanks for your contributions. I appreciate your work.

When something will be checked multiple times (such as the array length in a loop) and the value will not change, it's best to store that value in a variable.

Why is that?

The obvious thing to point out is that if we do not store the length in a variable, the length has to be retrieved each time the loop is iterated. But does this cause a true performance penalty? Is the cost of array.length greater than the cost of a regular variable access? Even if the answer is Yes, it would be considered as a micro-optimization.

What if I change the array inside the loop, for example adding an element or removing an element... I would need to update the precalculated variable, and if I forget to do this I will get undesired results. So, if I want to be on the safe side I better check the actual length property.

I see this as too risky to be included in a style guide.

Anyway, I must tell you that I have seen this technique (storing the length in a variable) used in some JS code in the past. I am aware that this, at least, was a well-established practice among the first JS programmers, and I believe that it was because JavaScript Virtual Machines (V8 et al.) were not optimizing these cases. I would like to know if the situation has changed or not.

For example, the Mozilla site is not using this approach.

image

This has been extracted from the Angular.js source code:

image

If you would like to experiment with it, you can write your tests in jsperf.com to see if this really makes a difference nowadays. I would love to see your results!

@GabrieleRomeo
Copy link
Contributor

Hi @jllodra and @meowwwls

there exist already some for-loop test cases on JsPerf. Have a look at this, please:

https://jsperf.com/for-loop-research/80

The following picture shows the result of my test.

It seems the caching length outside loop is fastest than the classic for-loop.

However, I think the really impressing fact is the ++i form. Isn't it????

Is not the fastest one but it is 1.24% fastest which is an interesting result for me.

image

@jllodra
Copy link
Contributor

jllodra commented Oct 27, 2016

@GabrieleRomeo @meowwwls,

Yep, thank you for the quick research, I see there are still some subtle differences in benchmarks.

However, I think the really impressing fact is the ++i form. Isn't it????

Yes, when I see results like these I often think that the benchmarking technique applied is not correct at all. Why in the world ++i should be faster than i++? Maybe it is creating some sort of temp variable for i++ and not for ++i, but again it does not make sense. :)

I would love to talk to a V8 or SpiderMonkey/Chakra engineer to ask him why is that.

Indeed, considering the benchmarks are correct, if we had to write code for real-time software which usually has a "hot path" or "hot code" that is executed many times (for example, a 3d engine is always repainting all faces/triangles. And a sound engine is always decoding chunks of audio...), we would use the fastest approach, but it is still a micro-optimization, and I think a style guide is not the best place for a recommendation like this, because unless necessary, no loop should be written this way because it is not easier to read and, as I said if we updated the array inside the loop (pushing one element) we should take care of the len variable.

In Learn.MD you will see learn about the cost of algorithms and you will be able to differentiate between optimizations and micro-optimizations.

I am sorry I think I cannot recommend writing for-loops this way to all students for all situations (as a dogma) and I do not write for-loops this way either, but if you feel more comfortable writing for-loops this way, please do it.

@GabrieleRomeo
Copy link
Contributor

@jllodra got It, and I agree with you.
Cool!
👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants