Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Commit

Permalink
perf(compiler): An option to disable the ElementProbe.
Browse files Browse the repository at this point in the history
Closes #1118
Closes #1131
  • Loading branch information
jbdeboer committed Jun 9, 2014
1 parent c6fcc74 commit 9f0c7bc
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 30 deletions.
1 change: 1 addition & 0 deletions lib/core/module.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export "package:angular/core_dom/module_internal.dart" show
BrowserCookies,
Cache,
Compiler,
CompilerConfig,
Cookies,
BoundViewFactory,
DirectiveMap,
Expand Down
15 changes: 15 additions & 0 deletions lib/core_dom/compiler_config.dart
Original file line number Diff line number Diff line change
@@ -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});
}
29 changes: 20 additions & 9 deletions lib/core_dom/element_binder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand Down
11 changes: 7 additions & 4 deletions lib/core_dom/element_binder_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,30 @@ 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.
ElementBinderBuilder builder(FormatterMap formatters) =>
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);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/core_dom/module_internal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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';
Expand Down Expand Up @@ -60,6 +61,7 @@ class CoreDomModule extends Module {
bind(AttrMustache);

bind(Compiler, toImplementation: TaggingCompiler);
bind(CompilerConfig);

bind(ComponentFactory, toImplementation: ShadowDomComponentFactory);
bind(ShadowDomComponentFactory);
Expand Down
28 changes: 20 additions & 8 deletions lib/core_dom/shadow_dom_component_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<_ComponentAssetKey, async.Future<dom.StyleElement>> _styleElementCache = {};

Expand All @@ -53,7 +54,8 @@ class ShadowDomComponentFactory implements ComponentFactory {
injector.getByKey(COMPONENT_CSS_REWRITER_KEY),
_expando,
baseCss,
_styleElementCache);
_styleElementCache,
_config);
var controller = componentFactory.call(injector, scope, viewCache, http, templateCache,
directives);

Expand Down Expand Up @@ -81,6 +83,7 @@ class _ComponentFactory implements Function {
_styleElementCache;
final ComponentCssRewriter componentCssRewriter;
final WebPlatform platform;
final CompilerConfig _config;

dom.ShadowRoot shadowDom;
Scope shadowScope;
Expand All @@ -89,7 +92,7 @@ class _ComponentFactory implements Function {

_ComponentFactory(this.element, this.typeKey, this.component, this.treeSanitizer,
this.platform, this.componentCssRewriter, this._expando,
this._baseCss, this._styleElementCache);
this._baseCss, this._styleElementCache, this._config);

dynamic call(Injector injector, Scope scope,
ViewCache viewCache, Http http, TemplateCache templateCache,
Expand Down Expand Up @@ -187,12 +190,21 @@ 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);
..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);
probe = _expando[shadowDom] = new ElementProbe(
injector.getByKey(ELEMENT_PROBE_KEY), shadowDom, shadowInjector, shadowScope);
shadowScope.on(ScopeEvent.DESTROY).listen((ScopeEvent) {_expando[shadowDom] = null;});

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;
}
}
Expand Down
10 changes: 7 additions & 3 deletions lib/core_dom/transcluding_component_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions test/angular_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
51 changes: 45 additions & 6 deletions test/core_dom/compiler_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,24 @@ forBothCompilers(fn) {
m.bind(Compiler, toImplementation: WalkingCompiler);
return m;
});
fn();
fn('walking');
});

describe('tagging compiler', () {
beforeEachModule((Module m) {
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');
});
}

Expand All @@ -31,12 +40,12 @@ forAllCompilersAndComponentFactories(fn) {

return m;
});
fn();
fn('transcluding');
});
}

void main() {
forBothCompilers(() =>
forBothCompilers((compilerType) =>
describe('TranscludingComponentFactory', () {
TestBed _;

Expand All @@ -59,7 +68,7 @@ void main() {
}));
}));

forAllCompilersAndComponentFactories(() =>
forAllCompilersAndComponentFactories((compilerType) =>
describe('dte.compiler', () {
TestBed _;

Expand Down Expand Up @@ -357,6 +366,8 @@ void main() {
}));

it('should store ElementProbe with Elements', async(() {
if (compilerType == 'tagging-no-elementProbe') return;

_.compile('<div><simple>innerText</simple></div>');
microLeap();
_.rootScope.apply();
Expand All @@ -368,14 +379,40 @@ 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);
expect(shadowProbe.parent.element).toEqual(simpleElement);
}
}));

describe('elementProbeEnabled option', () {
beforeEachModule((Module m) {
m.bind(CompilerConfig, toValue:
new CompilerConfig.withOptions(elementProbeEnabled: false));
});

it('should not store ElementProbe with Elements', async(() {
_.compile('<div><simple>innerText</simple></div>');
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'<div>{{name}}:<simple>{{name}}</simple></div>');
Expand Down Expand Up @@ -737,6 +774,8 @@ void main() {
}));

describe('expando memory', () {
if (compilerType == 'tagging-no-elementProbe') return;

Expando expando;

beforeEach(inject((Expando _expando) => expando = _expando));
Expand Down

0 comments on commit 9f0c7bc

Please sign in to comment.