Optimizing Nested For Loop

Question

I'm working on a performance-intensive library and wanted to see if anyone had some ideas on how to improve the performance of this method which converts our models to a js object.

You'll see below that I've tried to employ a couple of optimization techniques based on my reading:

Use straight for loops to iterate objects, arrays
Cache nested objects to avoid excessive lookups (field = fields[i], etc).

Here's the method I'm trying to further optimize:

toObject: function() {

  var i, l, fields, field, schema, obj = {};

  for(i = 0, fields = Object.keys(this._schema), l = fields.length; i < l; i++) {

    field = fields[i], schema = this._schema[field];

    if(!this._data.hasOwnProperty(field) || (schema.hasOwnProperty('output') && !schema[field].output)) {
      continue;
    }

    if(schema.hasOwnProperty('collection')) {

      obj[field] = [];

      schema.collection.instances.forEach(function (mdl) {
        obj[field].push(mdl.toObject());
      });
    }

    obj[field] = schema.hasOwnProperty('processOut') ? schema.processOut.call(this, this._data[field]) : this._data[field];
  }

  return obj;
}

In particular, I'm wondering if there's a way to optimize:

schema.collection.instances.forEach(function (mdl) {
  obj[field].push(mdl.toObject());
});

If I'm not mistaken, the function within the forEach is being created on each iteration. I was going to try and move it out of the main for loop, but then I lose access to field which I need to set the property key on obj.

I also thought about turning this into another for/loop, but then I'd have to create another set of variables like so:

// these vars would be init'd at the top of toObject or 
// maybe it makes sense to put them within the parent 
// for loop to avoid the additional scope lookup?
var x, xl;

for(x = 0, xl = schema.collection.instances.length; x < xl; x++) {
  obj[field].push(schema.collection.instances[x].toObject());
}

This just looks a little ugly, though, to be honest - this is a situation where we are collectively willing to forgo a little readability for performance.

I realize these may be minor micro-optimizations, but they've been shown to add up in my anecdotal experience when modeling several thousands of objects.

@elclanrs I've seen many a jsperf test showing how slow map is. — doremi
– doremi, Commented Jun 4, 2014 at 22:10
@JacobMattison not sure I follow. I can't control what get's passed to a forEach function - it's (item, index, array). — doremi
– doremi, Commented Jun 4, 2014 at 22:10
Are you having performance issues at this point? If not I would worry about correctness and readability first. map is ok. — elclanrs
– elclanrs, Commented Jun 4, 2014 at 22:11
100% in agreement with @elclanrs. There comes a point when optimization becomes micro-optimization, which is rarely necessary except in the case of definite bottle-necks. — Josh Beam
– Josh Beam, Commented Jun 4, 2014 at 22:17

Bergi · Accepted Answer · 2014-06-04 22:39:51Z

1

The for loop you have suggested is about as good as you're going to get. A couple optimizations you could take would be to avoid property lookups:

var objval = obj[field],
    instances = schema.collection.instances;
for(x = 0, xl = instances.length; x < xl; ++x) {
  objval.push(instances[x].toObject());
}

On a semi-related note, hasOwnProperty() does cause a signficant performance hit (compared to something simpler like field !== undefined).

edited Jun 4, 2014 at 22:39

Bergi

671k162 gold badges1k silver badges1.5k bronze badges

answered Jun 4, 2014 at 22:33

mscdex

107k15 gold badges201 silver badges159 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Esailija Over a year ago

Because the size of the array is known he can use new Array(instances.length), instead of [] and push

mscdex Over a year ago

Last I benchmarked, using [] and push() was actually faster in the version of v8 that node (at least v0.10.x) uses.

M.Stramm Over a year ago

Hopefully I didn't benchmark wrong: jsperf.com/array-initialization-push-vs-indexer Current browsers agree that [i]=x is faster than push(x) and new Array(n) is faster than [] everywhere except Edge (which disguises itself as Chrome 46). You're welcome

Collectives™ on Stack Overflow

Optimizing Nested For Loop

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related