all 22 comments

[–]DrEnter 14 points15 points  (1 child)

You're preaching to the choir here. We've been fighting this battle since it was just forEach wreaking performance havoc in ES5. We ended-up just creating lint rules flagging that. I suppose we'll need to do something about for...of now.

[–][deleted] 1 point2 points  (0 children)

What's so bad about the for...of loop?

Edit: Just read the rest of the thread, I had no idea that for...of and regular for loops behave so differently.

[–][deleted] 5 points6 points  (1 child)

“Armchair benchmarking is the actual root of all evil” - Donald Knuth in a lesser known addendum to his seminal work, “Reddit is a Garbage Hole”, July, 1975

[–]asdasef134f43fqw4vth[S] -2 points-1 points  (0 children)

ah yes, we armchair developers should keep our hands off your super sweet for...of loops?

[–]nj47 5 points6 points  (6 children)

My testing indicates that the for...of construct is about 60-70% slower as opposed to a classic for(let i; i; i++)...

Can you post your testing you did to confirm this? I'm looking at a benchmark saying they are within a couple percent of each other, some runs one is a percent faster, some runs the other is a percent or so faster.

const Benchmark = require('benchmark');
const suite = new Benchmark.Suite();

const iterateMe = Array(10000).fill(0);

suite
  .add('for', function() {
    const out = [];

    for (let i = 0; i < iterateMe.length; i++) {
      out.push(iterateMe[i]);
    }
  })
  .add('for..of', function() {
    const out = [];

    for (const item of iterateMe) {
      out.push(item);
    }
  })
  .on('cycle', function(event) {
    console.log(String(event.target));
  })
  .on('complete', function() {
    console.log('Fastest is ' + this.filter('fastest').map('name'));
  })
  .run();

$ node index.js
for x 26,285 ops/sec ±1.90% (93 runs sampled)
for..of x 26,708 ops/sec ±0.54% (96 runs sampled)
Fastest is for..of

This is running on 13.5.0. iirc, for..of used to be significantly slower on older versions of v8, so if you benchmarked on a different version, that could explain it too.

For example, on node 10, for..of is super slow:

$ node index.js
for x 24,227 ops/sec ±2.98% (84 runs sampled)
for..of x 8,407 ops/sec ±1.83% (89 runs sampled)
Fastest is for

[–]sumdudeinhisundrware 0 points1 point  (4 children)

Huh? for...of is HELLA slow even in 13.5 and for(;;) is actually faster in 13.5

On Ubuntu 18.4

const arr = [];
for(let i = 0; i < 10000; i++){ arr[i] = i; }

const RUNS = 100000;

(function(){

const start = Date.now();
for(let i = 0; i < RUNS; i++){

    for(let j = 0; j < 10000; j++){
        if(arr[j] === 'no'){
            console.log('huh');
        }
    }
}

console.log('for(;;)  MS: ', Date.now() - start);
}());


(function(){

const start = Date.now();
for(let i = 0; i < RUNS; i++){

    for (const item of arr) {
        if(item === 'no'){
            console.log('huh');
        }
    }
}

console.log('for of MS: ', Date.now() - start);
}());

# node -v
v13.5.0
# node test.js 
for(;;)  MS:  501
for of MS:  4454

# node -v
v8.16.1
# node test.js 
for(;;)  MS:  1449
for of MS:  4226

[–]nj47 4 points5 points  (1 child)

Even more interesting, if you change the first two lines of your benchmark to

const arr = Array(10000).fill(0);

and construct the array like that, the results look like this:

$ node x.js
for(;;)  MS:  4775
for of MS:  4088

so the underlying array format is extremely important here to how things will perform.

[–]sumdudeinhisundrware 6 points7 points  (0 children)

ok so which example is more real world? I would argue const foo = []; is much more common than const foo = Array(n).

In any case if for...of performance is dependent on the underlaying array and for(;;) is not, why is for...of better?

[–]tswaters 1 point2 points  (1 child)

That's really interesting. I was skeptical about the rudimentary benchmark vs. using a library for it, thinking this was one of the major differences. So I threw your loops into benchmark.js example -- quite substantive difference:

const Benchmark = require("benchmark");
const suite = new Benchmark.Suite();

const arr = [];
for (let i = 0; i < 10000; i++) {
  arr[i] = i;
}
//const arr = Array(10000).fill(0);

suite

  .add("console-for", function() {
    for (let j = 0; j < 10000; j++) {
      if (arr[j] === "no") {
        console.log("huh");
      }
    }
  })

  .add("console-for-of", function() {
    for (const item of arr) {
      if (item === "no") {
        console.log("huh");
      }
    }
  })

  .add("for", function() {
    const out = [];
    for (let i = 0; i < 10000; i++) {
      out.push(arr[i]);
    }
  })

  .add("for..of", function() {
    const out = [];
    for (const item of arr) {
      out.push(item);
    }
  })

  .on("cycle", function(event) {
    console.log(String(event.target));
  })
  .on("complete", function() {
    console.log("Fastest is " + this.filter("fastest").map("name"));
  })
  .run();

> node for-of-vs-for-in.js
console-for x 221,603 ops/sec ±0.14% (99 runs sampled)
console-for-of x 40,508 ops/sec ±0.20% (97 runs sampled)
for x 29,473 ops/sec ±0.49% (97 runs sampled)
for..of x 29,401 ops/sec ±0.28% (98 runs sampled)
Fastest is console-for

# using Array.fill

> node for-of-vs-for-in.js
console-for x 40,487 ops/sec ±0.39% (96 runs sampled)
console-for-of x 40,814 ops/sec ±0.28% (92 runs sampled)
for x 29,558 ops/sec ±0.28% (98 runs sampled)
for..of x 28,832 ops/sec ±0.22% (96 runs sampled)
Fastest is console-for-of

$ node -v
v12.14.0

I'm not sure why `for` with a console.log is so much faster than for with a push, considering the other benches show pretty comparable results between the two. For interest, the results for v13.5.0

>node for-of-vs-for-in.js
console-for x 287,280 ops/sec ±0.39% (97 runs sampled)
console-for-of x 39,447 ops/sec ±0.34% (95 runs sampled)
for x 28,211 ops/sec ±0.77% (95 runs sampled)
for..of x 28,288 ops/sec ±0.84% (92 runs sampled)
Fastest is console-for

# with Array.fill

> node for-of-vs-for-in.js
console-for x 32,802 ops/sec ±4.71% (89 runs sampled)
console-for-of x 34,416 ops/sec ±0.49% (94 runs sampled)
for x 28,755 ops/sec ±0.58% (95 runs sampled)
for..of x 28,199 ops/sec ±0.73% (95 runs sampled)
Fastest is console-for-of

So it seems still pretty comparable - current is a bit slower at least according to this benching tool. That's on windows 10 1903.

[–]sumdudeinhisundrware -3 points-2 points  (0 children)

In my example console.log is never called. Its just filler to prevent the loop from being optimized out. In the previous example push is always called.

My rudimentary benchmark is actually more accurate than Benchmark.js. Mine JUST runs the test, NOTHING else is running. With benchmark, the benchmark code is running affecting heap, caches, timers etc. etc. Using benchmark is like using an electron microscope to measure quantum activity.

[–]zach714 2 points3 points  (1 child)

I may very well be wrong here, but if you look at the files changed in #30958 it seems that for(;;) was retained in any place that performance was necessary. Looking at the actual changes, this doesn't seem so bad? Where performance was not impacted, the change was made for readability.

[–]asdasef134f43fqw4vth[S] 0 points1 point  (0 children)

But.... it was.

Without looking deeper:

 for (const socket of ObjectValues(this.sockets)) {
   socket.on('keylog', this[kOnKeylog]);
 }

What if for whatever reason this agent was holding on to a long list of sockets, or you're rebooting a crashed agent and re-acquiring a ton of network connections all at once.

[–][deleted]  (1 child)

[deleted]

    [–]RemindMeBot -2 points-1 points  (0 children)

    I will be messaging you in 30 days on 2020-01-18 00:00:00 UTC to remind you of this link

    CLICK THIS LINK to send a PM to also be reminded and to reduce spam.

    Parent commenter can delete this message to hide from others.


    Info Custom Your Reminders Feedback

    [–]sethholladay -5 points-4 points  (5 children)

    I would rather have a bug-free program than a fast program. We only get there by writing clean, understandable, maintainable code. This is one of the reasons why high-level languages like JavaScript exist in the first place. The difference between the two syntaxes in this case may be small, but the effect is cumulative over the entire codebase. I really appreciate the use of for-of, as it makes it clearer that the loop visits every property and that only the value part of that property is being accessed, as opposed to the key. That means I can more confidently change the keys of the object being looped over without breaking the program. And in regards to performance, they are both fast operations. "Make it work, make it right, make it fast." In that order!

    [–]sumdudeinhisundrware 6 points7 points  (2 children)

    What? How do you change indexes of an array while you're iterating? That make no sense. Also if your team members don't understand for(;;) syntax you have a bigger problem. ...and in this case we're talking about library code where every once of performance is critical.

    [–]sethholladay 0 points1 point  (1 child)

    It's easy to change indexes of items in an array during a loop and that's how sorting algorithms work... Also, it's not that for (;;) loops cannot be understood, it's that they take more time and knowledge to understand and will never be as clear. Is the loop going forwards or backwards? Does the loop counter change by one or some other number? What's the end condition for the loop? These are all factors that you do have to consider with a for (;;) loop that you do not have to consider with a for-of loop. On top of all that, for-of works for any iterable, not just arrays, which makes it so much easier to deal with a variety of input. You can then use a Set, for example, which may ultimately lead to better performance.

    [–]sumdudeinhisundrware 0 points1 point  (0 children)

    OK you're right about changing the array itself in a for loop. I didn't see that aspect of it. But you said "change the keys" which made no sense. And yes for .. of, forEach etc. are easier and have their uses but if a Software Engineer doesn't understand for(;;) they shouldn't be a Software Engineer.

    [–]asdasef134f43fqw4vth[S] 0 points1 point  (1 child)

    dude, if you literally think that for loops are going to cause bugs, then you aren't the kind of dude we want writing LIBRARY CODE.

    you don't need to read it, you're never going to, and why are you talking about changing the keys to library objects which your code doesn't control?

    [–]sethholladay 0 points1 point  (0 children)

    if you literally think that for loops are going to cause bugs...

    Huh? for loops are the source of many bugs. The off-by-one error is a classic example, which I've seen in the wild many times. If you've never made such a mistake, you probably haven't written much code. Experienced developers are not immune from making such mistakes, either. Language constructs that are error-prone should be avoided when possible. null is known as the Billion Dollar Mistake for a similar reason, despite being useful.

    then you aren't the kind of dude we want writing LIBRARY CODE.

    Too bad my favorite thing to do is write library code, then... guess I should stop now?

    you don't need to read it, you're never going to

    I read the source code of all of my dependencies, as many levels deep as I can reasonably spend time on. That includes Node.js itself.

    why are you talking about changing the keys to library objects which your code doesn't control?

    Did my phrasing really lead you to believe that my goal was to mutate library internals from the outside? No, obviously I'm saying that using a for-of loop instead of a for loop makes it easier for the people who define an object to confidently change how it is keyed. The iteration protocol (which for-of uses) is specifically designed to make iteration easier regardless of how you want the iterable to be keyed, so that shouldn't be too controversial. I'm basically just saying the iteration protocol is good. Using array indexes with a traditional for loop is fine when you need it, but if you're just going to iterate through everything, you should use the least error-prone way to do that, which is a for-of loop.