From 21d2dc6420d87efa81b7bc984e57219fc309d56f Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Wed, 22 Oct 2014 01:00:56 -0700 Subject: [PATCH 1/4] perf(benchmark): add ngModel largetable case --- benchmarks/largetable-bp/main.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/benchmarks/largetable-bp/main.html b/benchmarks/largetable-bp/main.html index 471a4441dd9e..b4f90a4cc951 100644 --- a/benchmarks/largetable-bp/main.html +++ b/benchmarks/largetable-bp/main.html @@ -20,6 +20,7 @@
interpolation + fnInvocation:
ngBind + filter:
interpolation + filter:
+
ngModel:
@@ -84,6 +85,13 @@

interpolation with filter

{{column.i | noop}}:{{column.j | noop}}| +
+

ngModels

+
+ + +
+
From 7a1eddab67d10998d09f223ae757f0e58a78d2e0 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 23 Oct 2014 00:27:20 -0700 Subject: [PATCH 2/4] refactor($compile): combining elementControllers and controllers --- src/ng/compile.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index b1d4185f6d23..71d9ea01581c 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1614,7 +1614,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { var terminalPriority = -Number.MAX_VALUE, newScopeDirective, controllerDirectives = previousCompileContext.controllerDirectives, - controllers, newIsolateScopeDirective = previousCompileContext.newIsolateScopeDirective, templateDirective = previousCompileContext.templateDirective, nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective, @@ -1911,8 +1910,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (controllerDirectives) { - // TODO: merge `controllers` and `elementControllers` into single object. - controllers = {}; elementControllers = {}; forEach(controllerDirectives, function(directive) { var locals = { @@ -1938,8 +1935,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (!hasElementTranscludeDirective) { $element.data('$' + directive.name + 'Controller', controllerInstance.instance); } - - controllers[directive.name] = controllerInstance; }); } @@ -1954,14 +1949,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { isolateScope.$$isolateBindings, newIsolateScopeDirective, isolateScope); } - if (controllers) { + if (elementControllers) { // Initialize bindToController bindings for new/isolate scopes var scopeDirective = newIsolateScopeDirective || newScopeDirective; var bindings; var controllerForBindings; - if (scopeDirective && controllers[scopeDirective.name]) { + if (scopeDirective && elementControllers[scopeDirective.name]) { bindings = scopeDirective.$$bindings.bindToController; - controller = controllers[scopeDirective.name]; + controller = elementControllers[scopeDirective.name]; if (controller && controller.identifier && bindings) { controllerForBindings = controller; @@ -1970,7 +1965,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings, scopeDirective); } } - forEach(controllers, function(controller) { + forEach(elementControllers, function(controller) { var result = controller(); if (result !== controller.instance && controller === controllerForBindings) { @@ -1981,7 +1976,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings, scopeDirective); } }); - controllers = null; } // PRELINKING From 019673009ac809289bd52034b9991619fcdbed82 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 23 Oct 2014 00:18:10 -0700 Subject: [PATCH 3/4] perf($compile): simplifying/refactoring controller loops/methods --- src/ng/compile.js | 105 +++++++++++++++++++++++----------------------- 1 file changed, 53 insertions(+), 52 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 71d9ea01581c..b512c7c1f4cc 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1671,7 +1671,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (!directive.templateUrl && directive.controller) { directiveValue = directive.controller; - controllerDirectives = controllerDirectives || {}; + controllerDirectives = controllerDirectives || createMap(); assertNoDuplicate("'" + directiveName + "' controller", controllerDirectives[directiveName], directive, $compileNode); controllerDirectives[directiveName] = directive; @@ -1839,49 +1839,43 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { function getControllers(directiveName, require, $element, elementControllers) { - var value, retrievalMethod = 'data', optional = false; - var $searchElement = $element; - var match; - if (isString(require)) { - match = require.match(REQUIRE_PREFIX_REGEXP); - require = require.substring(match[0].length); - - if (match[3]) { - if (match[1]) match[3] = null; - else match[1] = match[3]; - } - if (match[1] === '^') { - retrievalMethod = 'inheritedData'; - } else if (match[1] === '^^') { - retrievalMethod = 'inheritedData'; - $searchElement = $element.parent(); - } - if (match[2] === '?') { - optional = true; - } + var i, value; - value = null; + if (typeof require === 'string') { + var match = require.match(REQUIRE_PREFIX_REGEXP); + var name = require.substring(match[0].length); + var type = match[1] || match[3]; - if (elementControllers && retrievalMethod === 'data') { - if (value = elementControllers[require]) { - value = value.instance; - } + //If only parents then start at the parent element + //Otherwise attempt getting the controller from elementControllers to avoid .data + if (type === '^^') { + $element = $element.parent(); + } else { + value = elementControllers && elementControllers[name]; + value = value && value.instance; } - value = value || $searchElement[retrievalMethod]('$' + require + 'Controller'); - if (!value && !optional) { + if (!value) { + var dataName = '$' + name + 'Controller'; + value = type ? $element.inheritedData(dataName) : $element.data(dataName); + } + + if (!value && match[2] !== '?') { throw $compileMinErr('ctreq', "Controller '{0}', required by directive '{1}', can't be found!", - require, directiveName); + name, directiveName); } + return value || null; - } else if (isArray(require)) { - value = []; - forEach(require, function(require) { - value.push(getControllers(directiveName, require, $element, elementControllers)); - }); } - return value; + + if (isArray(require)) { + value = new Array(i = require.length); + while (i--) { + value[i] = getControllers(directiveName, require[i], $element, elementControllers); + } + return value; + } } @@ -1910,32 +1904,37 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (controllerDirectives) { - elementControllers = {}; - forEach(controllerDirectives, function(directive) { + elementControllers = createMap(); + + // For directives with element transclusion the element is a comment, + // but jQuery .data doesn't support attaching data to comment nodes as it's hard to + // clean up (http://bugs.jquery.com/ticket/8335). + // Instead, we save the controllers for the element in a local hash and attach to .data + // later, once we have the actual element. + var controllerData = !hasElementTranscludeDirective && $element.data(); + + for (var directiveName in controllerDirectives) { + var directive = controllerDirectives[directiveName]; + var locals = { $scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope, $element: $element, $attrs: attrs, $transclude: transcludeFn - }, controllerInstance; + }; - controller = directive.controller; - if (controller == '@') { - controller = attrs[directive.name]; + var directiveController = directive.controller; + if (directiveController === '@') { + directiveController = attrs[directive.name]; } - controllerInstance = $controller(controller, locals, true, directive.controllerAs); + var controllerInstance = $controller(directiveController, locals, true, directive.controllerAs); - // For directives with element transclusion the element is a comment, - // but jQuery .data doesn't support attaching data to comment nodes as it's hard to - // clean up (http://bugs.jquery.com/ticket/8335). - // Instead, we save the controllers for the element in a local hash and attach to .data - // later, once we have the actual element. elementControllers[directive.name] = controllerInstance; - if (!hasElementTranscludeDirective) { - $element.data('$' + directive.name + 'Controller', controllerInstance.instance); + if (controllerData) { + controllerData['$' + directive.name + 'Controller'] = controllerInstance.instance; } - }); + } } if (newIsolateScopeDirective) { @@ -1965,7 +1964,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings, scopeDirective); } } - forEach(elementControllers, function(controller) { + // Initialize the controllers before linking + for (i in elementControllers) { + controller = elementControllers[i]; var result = controller(); if (result !== controller.instance && controller === controllerForBindings) { @@ -1975,7 +1976,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { initializeDirectiveBindings(scope, attrs, result, bindings, scopeDirective); } - }); + } } // PRELINKING From e2ed092a687a839fdb8454eec88cbada02b16d6e Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 23 Oct 2014 22:11:32 -0700 Subject: [PATCH 4/4] refactor($compile): moving controller setup into its own method --- src/ng/compile.js | 64 ++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index b512c7c1f4cc..f8c7367fa3cc 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1878,6 +1878,38 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } } + function setupControllers(scope, isolateScope, $element, attrs, transcludeFn, elementControllers) { + // For directives with element transclusion the element is a comment, + // but jQuery .data doesn't support attaching data to comment nodes as it's hard to + // clean up (http://bugs.jquery.com/ticket/8335). + // Instead, we save the controllers for the element in a local hash and attach to .data + // later, once we have the actual element. + var controllerData = !hasElementTranscludeDirective && $element.data(); + + for (var directiveName in controllerDirectives) { + var directive = controllerDirectives[directiveName]; + + var locals = { + $scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope, + $element: $element, + $attrs: attrs, + $transclude: transcludeFn + }; + + var directiveController = directive.controller; + if (directiveController === '@') { + directiveController = attrs[directive.name]; + } + + var controllerInstance = $controller(directiveController, locals, true, directive.controllerAs); + + elementControllers[directive.name] = controllerInstance; + if (controllerData) { + controllerData['$' + directive.name + 'Controller'] = controllerInstance.instance; + } + } + } + function nodeLinkFn(childLinkFn, scope, linkNode, $rootElement, boundTranscludeFn, thisLinkFn) { @@ -1904,37 +1936,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (controllerDirectives) { - elementControllers = createMap(); - - // For directives with element transclusion the element is a comment, - // but jQuery .data doesn't support attaching data to comment nodes as it's hard to - // clean up (http://bugs.jquery.com/ticket/8335). - // Instead, we save the controllers for the element in a local hash and attach to .data - // later, once we have the actual element. - var controllerData = !hasElementTranscludeDirective && $element.data(); - - for (var directiveName in controllerDirectives) { - var directive = controllerDirectives[directiveName]; - - var locals = { - $scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope, - $element: $element, - $attrs: attrs, - $transclude: transcludeFn - }; - - var directiveController = directive.controller; - if (directiveController === '@') { - directiveController = attrs[directive.name]; - } - - var controllerInstance = $controller(directiveController, locals, true, directive.controllerAs); - - elementControllers[directive.name] = controllerInstance; - if (controllerData) { - controllerData['$' + directive.name + 'Controller'] = controllerInstance.instance; - } - } + setupControllers(scope, isolateScope, $element, attrs, transcludeFn, elementControllers = createMap()); } if (newIsolateScopeDirective) {