Speed optimizations

[italomandara]

PHASE 1 - so following this test... https://jsbench.me/lbj8m1fwn3/1 I decided to change forEach loops to for loops (~13% faster) to see if there's some visible speed improvs, i know this will make the code look ugly, but in some cases the improvement is quite good (~400ms in my test case).

PHASE2 - I noticed that layoutItem() appends each item causing a forced reflow for each single masonry item (https://gist.github.com/paulirish/5d52fb081b3570c81e3a), so I modified the layout() method using fragments to avoid that https://developer.mozilla.org/en-US/docs/Web/API/Document/createDocumentFragment, there are major speed improvements (~4x faster in my test case) especially in a masonry with 200 elements or more

[desandro]

Thank you for this contribution!

Colcade was originally intended as a new library for me to explore using all ES5, like forEach over for loops. I prefer forEach where available over micro-optimizations. I'll have to pass on the Phase 1 stuff.

Phase 2 - this is a good idea. However, I'm not keen on the current implementation. Here's what I'm thinking:

proto.layoutItems = function( items ) {

  var columnFragments = {};

  items.forEach( function( item ) {
    var index = this.getColumnLayoutIndex();
    // add item to fragment
    var fragment = columnFragments[ index ];
    if ( !fragment ) {
      // create fragment if not already there
      fragment = columnFragments[ index ] = document.createDocumentFragment();
    }
    fragment.appendChild( item );

    this.updateColumnHeight( index, item );
  });

  // append fragments to columns
  for ( var index in columnFragments ) {
    var fragment = columnFragments[ index ];
    this.appendColumnNode( index, fragment );
  }
};

proto.getColumnLayoutIndex = function() {
  var minHeight = Math.min.apply( Math, this.columnHeights );
  var index = this.columnHeights.indexOf( minHeight );
  return index;
};

proto.updateColumnHeight = function( index, item ) {
  // at least 1px, if item hasn't loaded
  // Not exactly accurate, but it's cool
  this.columnHeights[ index ] += item.offsetHeight || 1;
};

proto.appendColumnNode = function( index, node ) {
  var column = this.activeColumns[ index ];
  column.appendChild( node );
};

proto.layoutItem = function( item ) {
  var index = this.getColumnLayoutIndex();
  this.updateColumnHeight( index, item );
  this.appendColumnNode( index, item );
};

What do you think?

[italomandara]

Brilliant, I tested it, still much faster than the master version.

[italomandara]

Thank you for reviewing my PR @desandro the branch has been updated

[italomandara]

bad news: now looks like the masonry layout is not calculated properly in some cases, I'm working on a fix but if you have any sugestions please let me know.

• [x] fix items offsetHeight always = 0
• [ ] fix items height not matching

[psiho]

just a small sidenote on benchmark posted. For loop IS faster, but your linked test does not fully represent it. You used console.log in the test loop and you should avoid it. It is more time consuming than loop itself so you're basically testing console.log speed and not loops speed. On my latest Chrome on Linux both you tests are of equal speed. Since I guess you're not using console.log in real app, I think this test is more appropriate: https://jsbench.me/1qj9tvmwuv/1 It gives me about 35-40% increase of speed.

[desandro]

Yeah, I'm finding the same bug with images. The benefit of moving each item in the DOM is that you can immediately use its measurements. I'll have to hold off on this feature unless we can find a solution.