Repository: ember-cli/ember-cli
Pull request #6453 (2016-11-22 21:29:15)

Avoid creating extraneous merge-trees.
This change does three main things:

* Ensure that we always use `lib/broccoli/merge-trees.js` over requiring `broccoli-merge-trees` directly.
* When `lib/broccoli/merge-trees.js` is called with a single input tree, simply return it (instead of creating a `broccoli-merge-tree` instance).
* When `mergeTrees` is called with an empty array, use a single `EMPTY_MERGE_TREES` constant instance for all invocations.  This prevents us from creating many useless operations to essentially support an "empty" tree.  It is possible that we might want to teach Broccoli itself about empty trees, but that is left as a future excercise.

The results of these changes even in a newly created empty app are fairly significant. Here are the counts of `tmp` directories created before and after:

* 952 - Before changes
* 274 - After changes

In a real app, these numbers would be even higher (due to addons at each level of nesting).

---

Credit goes to @stefanpenner for the idea and initial implementation (which was stolen from https://github.com/ember-cli/ember-cli/pull/6429).

stefanpenner: nice!

@rwjblue may complement this nicely https://github.com/broccolijs/broccoli-merge-trees/pull/43
rwjblue: @homu r+
homu: :pushpin: Commit 380c2ec has been approved by `rwjblue`

<!-- @homu r=rwjblue 380c2ec29a51eb1d445fdb64c0909204c656b071 -->
homu: :hourglass: Testing commit 380c2ec with merge 95d4533...
rwjblue: @homu r-
homu: :sunny: Test successful - [status](https://travis-ci.org/ember-cli/ember-cli/builds/178116238)
rwjblue: @stefanpenner - Updated with tests and better filtering when input array contains the `EMPTY_MERGE_TREE` constant (this reduced the number of `tmp` dirs further down to 274).

Patch:
diff --git a/lib/broccoli/ember-app.js b/lib/broccoli/ember-app.js
index 31f58671d..624d7df47 100644
--- a/lib/broccoli/ember-app.js
+++ b/lib/broccoli/ember-app.js
@@ -715,7 +715,7 @@ EmberApp.prototype._templatesTree = function() {
     trees.push(this.podTemplates());
   }
 
-  this._cachedTemplateTree = mergeTrees(trees.filter(Boolean), {
+  this._cachedTemplateTree = mergeTrees(trees, {
     annotation: 'TreeMerge (templates)'
   });
 
diff --git a/lib/broccoli/merge-trees.js b/lib/broccoli/merge-trees.js
index e56ab17f8..559ef6846 100644
--- a/lib/broccoli/merge-trees.js
+++ b/lib/broccoli/merge-trees.js
@@ -1,13 +1,42 @@
 'use strict';
 
 var upstreamMergeTrees = require('broccoli-merge-trees');
+var EMPTY_MERGE_TREE;
 
-module.exports = function(inputTree, options) {
-  options = options || {};
-  options.description = options.annotation;
-  var tree = upstreamMergeTrees(inputTree, options);
+function overrideEmptyTree(tree) {
+  EMPTY_MERGE_TREE = tree;
+}
 
-  tree.description = options && options.description;
+function getEmptyTree() {
+  if (EMPTY_MERGE_TREE) {
+    return EMPTY_MERGE_TREE;
+  }
 
-  return tree;
+  return EMPTY_MERGE_TREE = upstreamMergeTrees([], {
+    annotation: 'EMPTY_MERGE_TREE',
+    description: 'EMPTY_MERGE_TREE'
+  });
+}
+
+module.exports = function mergeTrees(_inputTrees, options) {
+  var inputTrees = _inputTrees.filter(function(input) {
+    return input && input !== getEmptyTree();
+  });
+
+  switch (inputTrees.length) {
+  case 0:
+    return getEmptyTree();
+  case 1:
+    return inputTrees[0];
+  default:
+    options = options || {};
+    options.description = options.annotation;
+    var tree = upstreamMergeTrees(inputTrees, options);
+
+    tree.description = options && options.description;
+
+    return tree;
+  }
 };
+
+module.exports._overrideEmptyTree = overrideEmptyTree;
diff --git a/lib/models/addon.js b/lib/models/addon.js
index 2f79cfd5a..ea11d9dd4 100644
--- a/lib/models/addon.js
+++ b/lib/models/addon.js
@@ -22,20 +22,10 @@ var AddonsFactory   = require('../models/addons-factory');
 var CoreObject = require('core-object');
 var Project = require('./project');
 
-var upstreamMergeTrees = require('broccoli-merge-trees');
+var mergeTrees = require('../broccoli/merge-trees');
 var Funnel     = require('broccoli-funnel');
 var walkSync   = require('walk-sync');
 
-function mergeTrees(inputTree, options) {
-  options = options || {};
-  options.description = options.annotation;
-  var tree = upstreamMergeTrees(inputTree, options);
-
-  tree.description = options && options.description;
-
-  return tree;
-}
-
 function warn(message) {
   if (this.ui) {
     this.ui.writeDeprecateLine(message);
@@ -347,7 +337,7 @@ var Addon = CoreObject.extend({
       trees.push(this.jshintAddonTree());
     }
 
-    return mergeTrees(trees.filter(Boolean), {
+    return mergeTrees(trees, {
       overwrite: true,
       annotation: 'Addon#treeFor (' + this.name + ' - ' + name + ')'
     });
@@ -515,7 +505,7 @@ var Addon = CoreObject.extend({
     var addonTree = this.compileAddon(tree);
     var stylesTree = this.compileStyles(this._treeFor('addon-styles'));
 
-    return mergeTrees([addonTree, stylesTree].filter(Boolean), {
+    return mergeTrees([addonTree, stylesTree], {
       annotation: 'Addon#treeForAddon(' + this.name + ')'
     });
   },


Test patch:
diff --git a/tests/unit/broccoli/merge-trees-test.js b/tests/unit/broccoli/merge-trees-test.js
new file mode 100644
index 000000000..f2984adb9
--- /dev/null
+++ b/tests/unit/broccoli/merge-trees-test.js
@@ -0,0 +1,69 @@
+/* global escape */
+
+'use strict';
+
+var fs         = require('fs');
+var expect     = require('chai').expect;
+var proxyquire = require('proxyquire');
+var td = require('testdouble');
+
+var MockUI = require('../../helpers/mock-ui');
+
+var mergeTreesStub;
+var mergeTrees = proxyquire('../../../lib/broccoli/merge-trees', {
+  'broccoli-merge-trees': function() {
+    return mergeTreesStub.apply(this, arguments);
+  }
+});
+
+describe('broccoli/merge-trees', function() {
+  beforeEach(function() {
+    mergeTreesStub = td.function();
+  });
+
+  it('returns the first item when merging single item array', function() {
+    var actual = mergeTrees(['foo']);
+
+    expect(actual).to.equal('foo');
+  });
+
+  it('returns a constant "empty tree" when passed an empty array', function() {
+    var expected = {};
+
+    mergeTrees._overrideEmptyTree(expected);
+
+    var first = mergeTrees([]);
+    var second = mergeTrees([]);
+
+    expect(first).to.equal(expected);
+    expect(second).to.equal(expected);
+  });
+
+  it('passes all inputTrees through when non-empty', function() {
+    var expected = ['foo', 'bar'];
+    var actual;
+
+    mergeTreesStub = function(inputTrees) {
+      actual = inputTrees;
+      return {};
+    };
+
+    mergeTrees(['foo', null, undefined, 'bar']);
+    expect(actual).to.deep.equal(expected);
+  });
+
+  it('filters out empty trees from inputs', function() {
+    var expected = ['bar', 'baz'];
+    var actual;
+
+    mergeTrees._overrideEmptyTree('foo');
+
+    mergeTreesStub = function(inputTrees) {
+      actual = inputTrees;
+      return {};
+    };
+
+    mergeTrees(['foo', 'bar', 'baz']);
+    expect(actual).to.deep.equal(expected);
+  });
+});