perf($compile): avoid needless overhead when wrapping text nodes

This commit positively affects performance in two main ways: 1, When wrapping text nodes in the compile step, we do not need the overhead of the `forEach` function versus a normal for loop since we do not make use of the closure for anything. 2. When actually wrapping the node, we can completely bypass jqLite which avoids several function calls and the overhead of cloning the wrapper node which we already know to be unique. Tests in applications show about an 83% decrease in time spent in this specific loop.
angular · Jan 19, 2016 · 92e4801 · 92e4801
1 parent 6a92e91
commit 92e4801
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 10 deletions.
diff --git a/src/.jshintrc b/src/.jshintrc
@@ -138,6 +138,7 @@
     "jqLiteInheritedData": false,
     "jqLiteBuildFragment": false,
     "jqLiteParseHTML": false,
+    "jqLiteWrapNode": false,
     "getBooleanAttrName": false,
     "getAliasedAttrName": false,
     "createEventHandler": false,

diff --git a/src/jqLite.js b/src/jqLite.js
@@ -254,6 +254,16 @@ function jqLiteParseHTML(html, context) {
   return [];
 }
 
+function jqLiteWrapNode(node, wrapper) {
+  var parent = node.parentNode;
+
+  if (parent) {
+    parent.replaceChild(wrapper, node);
+  }
+
+  wrapper.appendChild(node);
+}
+
 
 // IE9-11 has no method "contains" in SVG element and in Node.prototype. Bug #10259.
 var jqLiteContains = Node.prototype.contains || function(arg) {
@@ -946,12 +956,7 @@ forEach({
   },
 
   wrap: function(element, wrapNode) {
-    wrapNode = jqLite(wrapNode).eq(0).clone()[0];
-    var parent = element.parentNode;
-    if (parent) {
-      parent.replaceChild(wrapNode, element);
-    }
-    wrapNode.appendChild(element);
+    jqLiteWrapNode(element, jqLite(wrapNode).eq(0).clone()[0]);
   },
 
   remove: jqLiteRemove,

diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -1501,13 +1501,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
         // modify it.
         $compileNodes = jqLite($compileNodes);
       }
+
+      var NOT_EMPTY = /\S+/;
+
       // We can not compile top level text elements since text nodes can be merged and we will
       // not be able to attach scope data to them, so we will wrap them in <span>
-      forEach($compileNodes, function(node, index) {
-        if (node.nodeType == NODE_TYPE_TEXT && node.nodeValue.match(/\S+/) /* non-empty */) {
-          $compileNodes[index] = jqLite(node).wrap('<span></span>').parent()[0];
+      for (var i = 0, len = $compileNodes.length; i < len; i++) {
+        var domNode = $compileNodes[i];
+
+        if (domNode.nodeType === NODE_TYPE_TEXT && domNode.nodeValue.match(NOT_EMPTY) /* non-empty */) {
+          jqLiteWrapNode(domNode, $compileNodes[i] = document.createElement('span'));
         }
-      });
+      }
+
       var compositeLinkFn =
               compileNodes($compileNodes, transcludeFn, $compileNodes,
                            maxPriority, ignoreDirective, previousCompileContext);