Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 92e4801

Browse files
dchermanpetebacondarwin
authored andcommitted
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.
1 parent 6a92e91 commit 92e4801

File tree

3 files changed

+22
-10
lines changed

3 files changed

+22
-10
lines changed

src/.jshintrc

+1
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@
138138
"jqLiteInheritedData": false,
139139
"jqLiteBuildFragment": false,
140140
"jqLiteParseHTML": false,
141+
"jqLiteWrapNode": false,
141142
"getBooleanAttrName": false,
142143
"getAliasedAttrName": false,
143144
"createEventHandler": false,

src/jqLite.js

+11-6
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,16 @@ function jqLiteParseHTML(html, context) {
254254
return [];
255255
}
256256

257+
function jqLiteWrapNode(node, wrapper) {
258+
var parent = node.parentNode;
259+
260+
if (parent) {
261+
parent.replaceChild(wrapper, node);
262+
}
263+
264+
wrapper.appendChild(node);
265+
}
266+
257267

258268
// IE9-11 has no method "contains" in SVG element and in Node.prototype. Bug #10259.
259269
var jqLiteContains = Node.prototype.contains || function(arg) {
@@ -946,12 +956,7 @@ forEach({
946956
},
947957

948958
wrap: function(element, wrapNode) {
949-
wrapNode = jqLite(wrapNode).eq(0).clone()[0];
950-
var parent = element.parentNode;
951-
if (parent) {
952-
parent.replaceChild(wrapNode, element);
953-
}
954-
wrapNode.appendChild(element);
959+
jqLiteWrapNode(element, jqLite(wrapNode).eq(0).clone()[0]);
955960
},
956961

957962
remove: jqLiteRemove,

src/ng/compile.js

+10-4
Original file line numberDiff line numberDiff line change
@@ -1501,13 +1501,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
15011501
// modify it.
15021502
$compileNodes = jqLite($compileNodes);
15031503
}
1504+
1505+
var NOT_EMPTY = /\S+/;
1506+
15041507
// We can not compile top level text elements since text nodes can be merged and we will
15051508
// not be able to attach scope data to them, so we will wrap them in <span>
1506-
forEach($compileNodes, function(node, index) {
1507-
if (node.nodeType == NODE_TYPE_TEXT && node.nodeValue.match(/\S+/) /* non-empty */) {
1508-
$compileNodes[index] = jqLite(node).wrap('<span></span>').parent()[0];
1509+
for (var i = 0, len = $compileNodes.length; i < len; i++) {
1510+
var domNode = $compileNodes[i];
1511+
1512+
if (domNode.nodeType === NODE_TYPE_TEXT && domNode.nodeValue.match(NOT_EMPTY) /* non-empty */) {
1513+
jqLiteWrapNode(domNode, $compileNodes[i] = document.createElement('span'));
15091514
}
1510-
});
1515+
}
1516+
15111517
var compositeLinkFn =
15121518
compileNodes($compileNodes, transcludeFn, $compileNodes,
15131519
maxPriority, ignoreDirective, previousCompileContext);

0 commit comments

Comments
 (0)