From c9ff35cdefab5efc0c53038ef1b2f53620b21634 Mon Sep 17 00:00:00 2001 From: Daniel Herman Date: Tue, 9 Jun 2015 18:36:59 -0400 Subject: [PATCH] perf($compile): Lazily compile the `transclude` function For transcluded directives, the transclude function can be lazily compiled most of the time since the contents will not be needed until the `transclude` function was actually invoked. For example, the `transclude` function that is passed to `ng-if` or `ng-switch-when` does not need to be invoked until the condition that it's bound to has been matched. For complex trees or switch statements, this can represent significant performance gains since compilation of branches is deferred, and that compilation may never actually happen if it isn't needed. There are two instances where compilation will not be lazy; when we scan ahead in the array of directives to be processed and find at least two of the following: * A directive that is transcluded and does not allow multiple transclusion * A directive that has templateUrl and replace: true * A directive that has a template and replace: true In both of those cases, we will need to continue eager compilation in order to generate the multiple transclusion exception at the correct time. --- src/ng/compile.js | 58 +++++++++++- test/ng/compileSpec.js | 207 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 263 insertions(+), 2 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 7a926f309fdc..b326d697907f 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1625,6 +1625,37 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { }; } + /** + * A function generator that is used to support both eager and lazy compilation + * linking function. + * @param eager + * @param $compileNodes + * @param transcludeFn + * @param maxPriority + * @param ignoreDirective + * @param previousCompileContext + * @returns {Function} + */ + function compilationGenerator(eager, $compileNodes, transcludeFn, maxPriority, ignoreDirective, previousCompileContext) { + if (eager) { + return compile($compileNodes, transcludeFn, maxPriority, ignoreDirective, previousCompileContext); + } + + var compiled; + + return function() { + if (!compiled) { + compiled = compile($compileNodes, transcludeFn, maxPriority, ignoreDirective, previousCompileContext); + + // Null out all of these references in order to make them eligible for garbage collection + // since this is a potentially long lived closure + $compileNodes = transcludeFn = previousCompileContext = null; + } + + return compiled.apply(this, arguments); + }; + } + /** * Once the directives have been collected, their compile functions are executed. This method * is responsible for inlining directive templates as well as terminating the application @@ -1669,6 +1700,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { replaceDirective = originalReplaceDirective, childTranscludeFn = transcludeFn, linkFn, + didScanForMultipleTransclusion = false, + mightHaveMultipleTransclusionError = false, directiveValue; // executes all directives on the current element @@ -1711,6 +1744,27 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { directiveName = directive.name; + // If we encounter a condition that can result in transclusion on the directive, + // then scan ahead in the remaining directives for others that may cause a multiple + // transclusion error to be thrown during the compilation process. If a matching directive + // is found, then we know that when we encounter a transcluded directive, we need to eagerly + // compile the `transclude` function rather than doing it lazily in order to throw + // exceptions at the correct time + if (!didScanForMultipleTransclusion && ((directive.replace && (directive.templateUrl || directive.template)) + || (directive.transclude && !directive.$$tlb))) { + var candidateDirective; + + for (var scanningIndex = i + 1; candidateDirective = directives[scanningIndex++];) { + if ((candidateDirective.transclude && !candidateDirective.$$tlb) + || (candidateDirective.replace && (candidateDirective.templateUrl || candidateDirective.template ))) { + mightHaveMultipleTransclusionError = true; + break; + } + } + + didScanForMultipleTransclusion = true; + } + if (!directive.templateUrl && directive.controller) { directiveValue = directive.controller; controllerDirectives = controllerDirectives || createMap(); @@ -1740,7 +1794,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { compileNode = $compileNode[0]; replaceWith(jqCollection, sliceArgs($template), compileNode); - childTranscludeFn = compile($template, transcludeFn, terminalPriority, + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn, terminalPriority, replaceDirective && replaceDirective.name, { // Don't pass in: // - controllerDirectives - otherwise we'll create duplicates controllers @@ -1754,7 +1808,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } else { $template = jqLite(jqLiteClone(compileNode)).contents(); $compileNode.empty(); // clear contents - childTranscludeFn = compile($template, transcludeFn); + childTranscludeFn = compilationGenerator(mightHaveMultipleTransclusionError, $template, transcludeFn); } } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 0239b8cd2cac..47f40da25adf 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -6596,6 +6596,27 @@ describe('$compile', function() { }); }); + it('should only allow one element transclusion per element when replace directive is in the mix', function() { + module(function() { + directive('template', valueFn({ + template: '

', + replace: true + })); + directive('first', valueFn({ + transclude: 'element', + priority: 100 + })); + directive('second', valueFn({ + transclude: 'element' + })); + }); + inject(function($compile) { + expect(function() { + $compile('
'); + }).toThrowMinErr('$compile', 'multidir', /Multiple directives \[first, second\] asking for transclusion on:

FooBar', + compile: function() { + innerCompilationCount +=1; + } + })); + }); + + inject(function($compile, $rootScope) { + element = $compile('')($rootScope); + expect(innerCompilationCount).toBe(0); + transclude(function(child) { element.append(child); }); + expect(innerCompilationCount).toBe(1); + expect(element.text()).toBe('FooBar'); + }); + }); + + it('should lazily compile the contents of directives that are transcluded with a template', function() { + var innerCompilationCount = 0, transclude; + + module(function() { + directive('trans', valueFn({ + transclude: true, + template: '

Baz
', + controller: function($transclude) { + transclude = $transclude; + } + })); + + directive('inner', valueFn({ + template: 'FooBar', + compile: function() { + innerCompilationCount +=1; + } + })); + }); + + inject(function($compile, $rootScope) { + element = $compile('')($rootScope); + expect(innerCompilationCount).toBe(0); + transclude(function(child) { element.append(child); }); + expect(innerCompilationCount).toBe(1); + expect(element.text()).toBe('BazFooBar'); + }); + }); + + it('should lazily compile the contents of directives that are transcluded with a templateUrl', function() { + var innerCompilationCount = 0, transclude; + + module(function() { + directive('trans', valueFn({ + transclude: true, + templateUrl: 'baz.html', + controller: function($transclude) { + transclude = $transclude; + } + })); + + directive('inner', valueFn({ + template: 'FooBar', + compile: function() { + innerCompilationCount +=1; + } + })); + }); + + inject(function($compile, $rootScope, $httpBackend) { + $httpBackend.expectGET('baz.html').respond('
Baz
'); + element = $compile('')($rootScope); + $httpBackend.flush(); + + expect(innerCompilationCount).toBe(0); + transclude(function(child) { element.append(child); }); + expect(innerCompilationCount).toBe(1); + expect(element.text()).toBe('BazFooBar'); + }); + }); + + it('should lazily compile the contents of directives that are transclude element', function() { + var innerCompilationCount = 0, transclude; + + module(function() { + directive('trans', valueFn({ + transclude: 'element', + controller: function($transclude) { + transclude = $transclude; + } + })); + + directive('inner', valueFn({ + template: 'FooBar', + compile: function() { + innerCompilationCount +=1; + } + })); + }); + + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + expect(innerCompilationCount).toBe(0); + transclude(function(child) { element.append(child); }); + expect(innerCompilationCount).toBe(1); + expect(element.text()).toBe('FooBar'); + }); + }); + + it('should lazily compile transcluded directives with ngIf on them', function() { + var innerCompilationCount = 0, outerCompilationCount = 0, transclude; + + module(function() { + directive('outer', valueFn({ + transclude: true, + compile: function() { + outerCompilationCount += 1; + }, + controller: function($transclude) { + transclude = $transclude; + } + })); + + directive('inner', valueFn({ + template: 'FooBar', + compile: function() { + innerCompilationCount +=1; + } + })); + }); + + inject(function($compile, $rootScope) { + $rootScope.shouldCompile = false; + + element = $compile('
')($rootScope); + expect(outerCompilationCount).toBe(0); + expect(innerCompilationCount).toBe(0); + expect(transclude).toBeUndefined(); + $rootScope.$apply('shouldCompile=true'); + expect(outerCompilationCount).toBe(1); + expect(innerCompilationCount).toBe(0); + expect(transclude).toBeDefined(); + transclude(function(child) { element.append(child); }); + expect(outerCompilationCount).toBe(1); + expect(innerCompilationCount).toBe(1); + expect(element.text()).toBe('FooBar'); + }); + }); + + it('should eagerly compile multiple directives with transclusion and templateUrl/replace', function() { + var innerCompilationCount = 0; + + module(function() { + directive('outer', valueFn({ + transclude: true + })); + + directive('outer', valueFn({ + templateUrl: 'inner.html', + replace: true + })); + + directive('inner', valueFn({ + compile: function() { + innerCompilationCount +=1; + } + })); + }); + + inject(function($compile, $rootScope, $httpBackend) { + $httpBackend.expectGET('inner.html').respond(''); + element = $compile('')($rootScope); + $httpBackend.flush(); + + expect(innerCompilationCount).toBe(1); + }); + }); });