From 40d59ef3e1c2bfdf3f5f06bc045ba12abb070195 Mon Sep 17 00:00:00 2001 From: James deBoer Date: Mon, 9 Jun 2014 11:49:33 -0700 Subject: [PATCH] perf(compiler): An option to disable the ElementProbe. Closes #1118 Closes #1131 --- lib/core/module.dart | 1 + lib/core_dom/compiler_config.dart | 15 ++++++ lib/core_dom/element_binder.dart | 29 +++++++---- lib/core_dom/element_binder_builder.dart | 11 ++-- lib/core_dom/module_internal.dart | 2 + .../shadow_dom_component_factory.dart | 30 +++++++---- .../transcluding_component_factory.dart | 10 ++-- test/angular_spec.dart | 1 + test/core_dom/compiler_spec.dart | 51 ++++++++++++++++--- 9 files changed, 119 insertions(+), 31 deletions(-) create mode 100644 lib/core_dom/compiler_config.dart diff --git a/lib/core/module.dart b/lib/core/module.dart index 57b7b2970..912101498 100644 --- a/lib/core/module.dart +++ b/lib/core/module.dart @@ -28,6 +28,7 @@ export "package:angular/core_dom/module_internal.dart" show BrowserCookies, Cache, Compiler, + CompilerConfig, Cookies, BoundViewFactory, DirectiveMap, diff --git a/lib/core_dom/compiler_config.dart b/lib/core_dom/compiler_config.dart new file mode 100644 index 000000000..3ef2b8895 --- /dev/null +++ b/lib/core_dom/compiler_config.dart @@ -0,0 +1,15 @@ +part of angular.core.dom_internal; + +/** + * Global configuration options for the Compiler + */ +@Injectable() +class CompilerConfig { + /** + * True if the compiler should add ElementProbes to the elements. + */ + final bool elementProbeEnabled; + + CompilerConfig() : elementProbeEnabled = true; + CompilerConfig.withOptions({this.elementProbeEnabled: true}); +} diff --git a/lib/core_dom/element_binder.dart b/lib/core_dom/element_binder.dart index 5f04f7e36..558b56c06 100644 --- a/lib/core_dom/element_binder.dart +++ b/lib/core_dom/element_binder.dart @@ -14,11 +14,11 @@ class TemplateElementBinder extends ElementBinder { return _directiveCache = [template]; } - TemplateElementBinder(perf, expando, parser, componentFactory, + TemplateElementBinder(perf, expando, parser, config, componentFactory, transcludingComponentFactory, shadowDomComponentFactory, this.template, this.templateBinder, onEvents, bindAttrs, childMode) - : super(perf, expando, parser, componentFactory, + : super(perf, expando, parser, config, componentFactory, transcludingComponentFactory, shadowDomComponentFactory, null, null, onEvents, bindAttrs, childMode); @@ -45,6 +45,7 @@ class ElementBinder { final Profiler _perf; final Expando _expando; final Parser _parser; + final CompilerConfig _config; // The default component factory final ComponentFactory _componentFactory; @@ -61,7 +62,7 @@ class ElementBinder { // Can be either COMPILE_CHILDREN or IGNORE_CHILDREN final String childMode; - ElementBinder(this._perf, this._expando, this._parser, + ElementBinder(this._perf, this._expando, this._parser, this._config, this._componentFactory, this._transcludingComponentFactory, this._shadowDomComponentFactory, @@ -207,7 +208,9 @@ class ElementBinder { void _link(nodeInjector, probe, scope, nodeAttrs) { _usableDirectiveRefs.forEach((DirectiveRef ref) { var directive = nodeInjector.getByKey(ref.typeKey); - probe.directives.add(directive); + if (probe != null) { + probe.directives.add(directive); + } if (ref.annotation is Controller) { scope.context[(ref.annotation as Controller).publishAs] = directive; @@ -295,8 +298,11 @@ class ElementBinder { ..bindByKey(VIEW_KEY, toValue: view) ..bindByKey(ELEMENT_KEY, toValue: node) ..bindByKey(NODE_KEY, toValue: node) - ..bindByKey(NODE_ATTRS_KEY, toValue: nodeAttrs) - ..bindByKey(ELEMENT_PROBE_KEY, toFactory: (_) => probe); + ..bindByKey(NODE_ATTRS_KEY, toValue: nodeAttrs); + + if (_config.elementProbeEnabled) { + nodeModule.bindByKey(ELEMENT_PROBE_KEY, toFactory: (_) => probe); + } directiveRefs.forEach((DirectiveRef ref) { Directive annotation = ref.annotation; @@ -316,9 +322,14 @@ class ElementBinder { _registerViewFactory(node, parentInjector, nodeModule); nodeInjector = parentInjector.createChild([nodeModule]); - probe = _expando[node] = new ElementProbe( - parentInjector.getByKey(ELEMENT_PROBE_KEY), node, nodeInjector, scope); - scope.on(ScopeEvent.DESTROY).listen((_) {_expando[node] = null;}); + if (_config.elementProbeEnabled) { + probe = _expando[node] = + new ElementProbe(parentInjector.getByKey(ELEMENT_PROBE_KEY), + node, nodeInjector, scope); + scope.on(ScopeEvent.DESTROY).listen((_) { + _expando[node] = null; + }); + } _link(nodeInjector, probe, scope, nodeAttrs); diff --git a/lib/core_dom/element_binder_builder.dart b/lib/core_dom/element_binder_builder.dart index 9f02551c0..343906364 100644 --- a/lib/core_dom/element_binder_builder.dart +++ b/lib/core_dom/element_binder_builder.dart @@ -4,14 +4,17 @@ part of angular.core.dom_internal; class ElementBinderFactory { final Parser _parser; final Profiler _perf; + final CompilerConfig _config; final Expando _expando; final ComponentFactory _componentFactory; final TranscludingComponentFactory _transcludingComponentFactory; final ShadowDomComponentFactory _shadowDomComponentFactory; final ASTParser _astParser; - ElementBinderFactory(this._parser, this._perf, this._expando, this._componentFactory, - this._transcludingComponentFactory, this._shadowDomComponentFactory, + ElementBinderFactory(this._parser, this._perf, this._config, this._expando, + this._componentFactory, + this._transcludingComponentFactory, + this._shadowDomComponentFactory, this._astParser); // TODO: Optimize this to re-use a builder. @@ -19,12 +22,12 @@ class ElementBinderFactory { new ElementBinderBuilder(this, _astParser, formatters); ElementBinder binder(ElementBinderBuilder b) => - new ElementBinder(_perf, _expando, _parser, _componentFactory, + new ElementBinder(_perf, _expando, _parser, _config, _componentFactory, _transcludingComponentFactory, _shadowDomComponentFactory, b.component, b.decorators, b.onEvents, b.bindAttrs, b.childMode); TemplateElementBinder templateBinder(ElementBinderBuilder b, ElementBinder transclude) => - new TemplateElementBinder(_perf, _expando, _parser, _componentFactory, + new TemplateElementBinder(_perf, _expando, _parser, _config, _componentFactory, _transcludingComponentFactory, _shadowDomComponentFactory, b.template, transclude, b.onEvents, b.bindAttrs, b.childMode); } diff --git a/lib/core_dom/module_internal.dart b/lib/core_dom/module_internal.dart index c40fb0499..5ae7662c8 100644 --- a/lib/core_dom/module_internal.dart +++ b/lib/core_dom/module_internal.dart @@ -26,6 +26,7 @@ part 'view_factory.dart'; part 'cookies.dart'; part 'common.dart'; part 'compiler.dart'; +part 'compiler_config.dart'; part 'directive.dart'; part 'directive_map.dart'; part 'element_binder.dart'; @@ -58,6 +59,7 @@ class CoreDomModule extends Module { bind(AttrMustache); bind(Compiler, toImplementation: TaggingCompiler); + bind(CompilerConfig); bind(ComponentFactory, toImplementation: ShadowDomComponentFactory); bind(ShadowDomComponentFactory); diff --git a/lib/core_dom/shadow_dom_component_factory.dart b/lib/core_dom/shadow_dom_component_factory.dart index 4dbbd3525..b5538d563 100644 --- a/lib/core_dom/shadow_dom_component_factory.dart +++ b/lib/core_dom/shadow_dom_component_factory.dart @@ -28,8 +28,9 @@ abstract class ComponentFactory { @Injectable() class ShadowDomComponentFactory implements ComponentFactory { final Expando _expando; + final CompilerConfig _config; - ShadowDomComponentFactory(this._expando); + ShadowDomComponentFactory(this._expando, this._config); final Map> _styleElementCache = {}; @@ -46,7 +47,7 @@ class ShadowDomComponentFactory implements ComponentFactory { NgBaseCss baseCss = component.useNgBaseCss ? injector.getByKey(NG_BASE_CSS_KEY) : null; // This is a bit of a hack since we are returning different type then we are. var componentFactory = new _ComponentFactory(node, ref.typeKey, component, - injector.getByKey(NODE_TREE_SANITIZER_KEY), _expando, baseCss, _styleElementCache); + injector.getByKey(NODE_TREE_SANITIZER_KEY), _expando, baseCss, _styleElementCache, _config); var controller = componentFactory.call(injector, scope, viewCache, http, templateCache, directives); @@ -71,6 +72,7 @@ class _ComponentFactory implements Function { final Expando _expando; final NgBaseCss _baseCss; final Map> _styleElementCache; + final CompilerConfig _config; dom.ShadowRoot shadowDom; Scope shadowScope; @@ -78,7 +80,7 @@ class _ComponentFactory implements Function { var controller; _ComponentFactory(this.element, this.typeKey, this.component, this.treeSanitizer, - this._expando, this._baseCss, this._styleElementCache); + this._expando, this._baseCss, this._styleElementCache, this._config); dynamic call(Injector injector, Scope scope, ViewCache viewCache, Http http, TemplateCache templateCache, @@ -141,12 +143,22 @@ class _ComponentFactory implements Function { ..bindByKey(EVENT_HANDLER_KEY, toImplementation: ShadowRootEventHandler) ..bindByKey(SCOPE_KEY, toValue: shadowScope) ..bindByKey(TEMPLATE_LOADER_KEY, toValue: templateLoader) - ..bindByKey(SHADOW_ROOT_KEY, toValue: shadowDom) - ..bindByKey(ELEMENT_PROBE_KEY, toFactory: (_) => probe); - shadowInjector = injector.createChild([shadowModule], name: SHADOW_DOM_INJECTOR_NAME); - probe = _expando[shadowDom] = new ElementProbe( - injector.getByKey(ELEMENT_PROBE_KEY), shadowDom, shadowInjector, shadowScope); - shadowScope.on(ScopeEvent.DESTROY).listen((ScopeEvent) {_expando[shadowDom] = null;}); + ..bindByKey(SHADOW_ROOT_KEY, toValue: shadowDom); + + if (_config.elementProbeEnabled) { + shadowModule.bindByKey(ELEMENT_PROBE_KEY, toFactory: (_) => probe); + } + + shadowInjector = injector.createChild([shadowModule], name: SHADOW_DOM_INJECTOR_NAME); + + if (_config.elementProbeEnabled) { + probe = _expando[shadowDom] = + new ElementProbe(injector.getByKey(ELEMENT_PROBE_KEY), + shadowDom, shadowInjector, shadowScope); + shadowScope.on(ScopeEvent.DESTROY).listen((ScopeEvent) { + _expando[shadowDom] = null; + }); + } return shadowInjector; } } diff --git a/lib/core_dom/transcluding_component_factory.dart b/lib/core_dom/transcluding_component_factory.dart index 2a534aab8..fcd2881c5 100644 --- a/lib/core_dom/transcluding_component_factory.dart +++ b/lib/core_dom/transcluding_component_factory.dart @@ -71,8 +71,9 @@ class ContentPort { @Injectable() class TranscludingComponentFactory implements ComponentFactory { final Expando _expando; + final CompilerConfig _config; - TranscludingComponentFactory(this._expando); + TranscludingComponentFactory(this._expando, this._config); FactoryFn call(dom.Node node, DirectiveRef ref) { // CSS is not supported. @@ -116,8 +117,11 @@ class TranscludingComponentFactory implements ComponentFactory { ..bind(ContentPort, toValue: contentPort) ..bind(Scope, toValue: shadowScope) ..bind(TemplateLoader, toValue: templateLoader) - ..bind(dom.ShadowRoot, toValue: new ShadowlessShadowRoot(element)) - ..bind(ElementProbe, toFactory: (_) => probe); + ..bind(dom.ShadowRoot, toValue: new ShadowlessShadowRoot(element)); + + if (_config.elementProbeEnabled) { + childModule.bind(ElementProbe, toFactory: (_) => probe); + } childInjector = injector.createChild([childModule], name: SHADOW_DOM_INJECTOR_NAME); var controller = childInjector.get(ref.type); diff --git a/test/angular_spec.dart b/test/angular_spec.dart index 638548bee..68fa07a8b 100644 --- a/test/angular_spec.dart +++ b/test/angular_spec.dart @@ -115,6 +115,7 @@ main() { "angular.core.dom_internal.BoundViewFactory", "angular.core.dom_internal.BrowserCookies", "angular.core.dom_internal.Compiler", + "angular.core.dom_internal.CompilerConfig", "angular.core.dom_internal.Cookies", "angular.core.dom_internal.DirectiveMap", "angular.core.dom_internal.ElementProbe", diff --git a/test/core_dom/compiler_spec.dart b/test/core_dom/compiler_spec.dart index fe9130549..97c19d325 100644 --- a/test/core_dom/compiler_spec.dart +++ b/test/core_dom/compiler_spec.dart @@ -9,7 +9,7 @@ forBothCompilers(fn) { m.bind(Compiler, toImplementation: WalkingCompiler); return m; }); - fn(); + fn('walking'); }); describe('tagging compiler', () { @@ -17,7 +17,16 @@ forBothCompilers(fn) { m.bind(Compiler, toImplementation: TaggingCompiler); return m; }); - fn(); + fn('tagging'); + }); + + describe('tagging compiler with ElementProbe disabled', () { + beforeEachModule((Module m) { + m.bind(Compiler, toImplementation: TaggingCompiler); + m.bind(CompilerConfig, toValue: new CompilerConfig.withOptions(elementProbeEnabled: false)); + return m; + }); + fn('tagging-no-elementProbe'); }); } @@ -31,12 +40,12 @@ forAllCompilersAndComponentFactories(fn) { return m; }); - fn(); + fn('transcluding'); }); } void main() { - forBothCompilers(() => + forBothCompilers((compilerType) => describe('TranscludingComponentFactory', () { TestBed _; @@ -59,7 +68,7 @@ void main() { })); })); - forAllCompilersAndComponentFactories(() => + forAllCompilersAndComponentFactories((compilerType) => describe('dte.compiler', () { TestBed _; @@ -357,6 +366,8 @@ void main() { })); it('should store ElementProbe with Elements', async(() { + if (compilerType == 'tagging-no-elementProbe') return; + _.compile('
innerText
'); microLeap(); _.rootScope.apply(); @@ -368,7 +379,7 @@ void main() { var shadowRoot = simpleElement.shadowRoot; // If there is no shadow root, skip this. - if (shadowRoot != null) { + if (compilerType != 'transcluding') { var shadowProbe = ngProbe(shadowRoot); expect(shadowProbe).toBeNotNull(); expect(shadowProbe.element).toEqual(shadowRoot); @@ -376,6 +387,32 @@ void main() { } })); + describe('elementProbeEnabled option', () { + beforeEachModule((Module m) { + m.bind(CompilerConfig, toValue: + new CompilerConfig.withOptions(elementProbeEnabled: false)); + }); + + it('should not store ElementProbe with Elements', async(() { + _.compile('
innerText
'); + microLeap(); + _.rootScope.apply(); + var simpleElement = _.rootElement.querySelector('simple'); + expect(simpleElement).toHaveText('INNER(innerText)'); + + expect(() => ngProbe(simpleElement)) + .toThrow("Could not find a probe for the node 'simple' nor its parents"); + + var shadowRoot = simpleElement.shadowRoot; + + // If there is no shadow root, skip this. + if (compilerType != 'transcluding') { + expect(() => ngProbe(shadowRoot)) + .toThrow("Could not find a probe for the node 'Instance of 'ShadowRoot'' nor its parents"); + } + })); + }); + it('should create a simple component', async((VmTurnZone zone) { _.rootScope.context['name'] = 'OUTTER'; var element = _.compile(r'
{{name}}:{{name}}
'); @@ -737,6 +774,8 @@ void main() { })); describe('expando memory', () { + if (compilerType == 'tagging-no-elementProbe') return; + Expando expando; beforeEach(inject((Expando _expando) => expando = _expando));