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

Commit 970611d

Browse files
committed
perf(view): increase view instantiation speed 40%
1) Stop using futures when the value is already cached 2) Remove adding nodes to fake parent during view instantiation Closes #1358
1 parent 0b0080b commit 970611d

File tree

9 files changed

+173
-133
lines changed

9 files changed

+173
-133
lines changed

lib/core_dom/directive.dart

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class NodeAttrs {
2222

2323
NodeAttrs(this.element);
2424

25-
operator [](String attrName) => element.attributes[attrName];
25+
operator [](String attrName) => element.getAttribute(attrName);
2626

2727
void operator []=(String attrName, String value) {
2828
if (_mustacheAttrs.containsKey(attrName)) {
@@ -31,7 +31,7 @@ class NodeAttrs {
3131
if (value == null) {
3232
element.attributes.remove(attrName);
3333
} else {
34-
element.attributes[attrName] = value;
34+
element.setAttribute(attrName, value);
3535
}
3636

3737
if (_observers != null && _observers.containsKey(attrName)) {
@@ -86,9 +86,18 @@ class NodeAttrs {
8686
* ShadowRoot is ready.
8787
*/
8888
class TemplateLoader {
89-
final async.Future<dom.Node> template;
89+
async.Future<dom.Node> _template;
90+
List<async.Future> _futures;
91+
final dom.Node _shadowRoot;
9092

91-
TemplateLoader(this.template);
93+
TemplateLoader(this._shadowRoot, this._futures);
94+
95+
async.Future<dom.Node> get template {
96+
if (_template == null) {
97+
_template = async.Future.wait(_futures).then((_) => _shadowRoot);
98+
}
99+
return _template;
100+
}
92101
}
93102

94103
class _MustacheAttr {

lib/core_dom/shadow_dom_component_factory.dart

Lines changed: 87 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ abstract class BoundComponentFactory {
1111
List<Key> get callArgs;
1212
Function call(dom.Element element);
1313

14-
static async.Future<ViewFactory> _viewFuture(
14+
static async.Future<ViewFactory> _viewFactoryFuture(
1515
Component component, ViewCache viewCache, DirectiveMap directives) {
1616
if (component.template != null) {
1717
return new async.Future.value(viewCache.fromHtml(component.template, directives));
@@ -66,20 +66,27 @@ class BoundShadowDomComponentFactory implements BoundComponentFactory {
6666
Component get _component => _ref.annotation as Component;
6767

6868
String _tag;
69-
async.Future<Iterable<dom.StyleElement>> _styleElementsFuture;
70-
async.Future<ViewFactory> _viewFuture;
69+
async.Future<List<dom.StyleElement>> _styleElementsFuture;
70+
List<dom.StyleElement> _styleElements;
71+
async.Future<ViewFactory> _shadowViewFactoryFuture;
72+
ViewFactory _shadowViewFactory;
7173

7274
BoundShadowDomComponentFactory(this._componentFactory, this._ref, this._directives, this._injector) {
7375
_tag = _component.selector.toLowerCase();
74-
_styleElementsFuture = async.Future.wait(_component.cssUrls.map(_styleFuture));
76+
_styleElementsFuture = async.Future.wait(_component.cssUrls.map(_urlToStyle))
77+
..then((stylesElements) => _styleElements = stylesElements);
7578

76-
_viewFuture = BoundComponentFactory._viewFuture(
79+
_shadowViewFactoryFuture = BoundComponentFactory._viewFactoryFuture(
7780
_component,
81+
// TODO(misko): Why do we create a new one per Component. This kind of defeats the caching.
7882
new PlatformViewCache(_componentFactory.viewCache, _tag, _componentFactory.platform),
7983
_directives);
84+
if (_shadowViewFactoryFuture != null) {
85+
_shadowViewFactoryFuture.then((viewFactory) => _shadowViewFactory = viewFactory);
86+
}
8087
}
8188

82-
async.Future<dom.StyleElement> _styleFuture(cssUrl) {
89+
async.Future<dom.StyleElement> _urlToStyle(cssUrl) {
8390
Http http = _componentFactory.http;
8491
TemplateCache templateCache = _componentFactory.templateCache;
8592
WebPlatform platform = _componentFactory.platform;
@@ -108,7 +115,7 @@ class BoundShadowDomComponentFactory implements BoundComponentFactory {
108115

109116
// If the css shim is required, it means that scoping does not
110117
// work, and adding the style to the head of the document is
111-
// preferrable.
118+
// preferable.
112119
if (platform.cssShimRequired) {
113120
dom.document.head.append(styleElement);
114121
return null;
@@ -127,50 +134,59 @@ class BoundShadowDomComponentFactory implements BoundComponentFactory {
127134
EventHandler _) {
128135
var s = traceEnter(View_createComponent);
129136
try {
130-
var shadowDom = element.createShadowRoot()
137+
var shadowScope = scope.createChild(new HashMap()); // Isolate
138+
ComponentDirectiveInjector shadowInjector;
139+
dom.ShadowRoot shadowRoot = element.createShadowRoot();
140+
shadowRoot
131141
..applyAuthorStyles = _component.applyAuthorStyles
132142
..resetStyleInheritance = _component.resetStyleInheritance;
133143

134-
var shadowScope = scope.createChild(new HashMap()); // Isolate
144+
List<async.Future> futures = <async.Future>[];
145+
TemplateLoader templateLoader = new TemplateLoader(shadowRoot, futures);
146+
var eventHandler = new ShadowRootEventHandler(
147+
shadowRoot, injector.getByKey(EXPANDO_KEY), injector.getByKey(EXCEPTION_HANDLER_KEY));
148+
shadowInjector = new ComponentDirectiveInjector(
149+
injector, this._injector, eventHandler, shadowScope, templateLoader, shadowRoot, null);
150+
shadowInjector.bindByKey(_ref.typeKey, _ref.factory, _ref.paramKeys,
151+
_ref.annotation.visibility);
152+
dom.Node firstViewNode = null;
153+
154+
// Load ngBase CSS
155+
if (_component.useNgBaseCss == true && baseCss.urls.isNotEmpty) {
156+
if (baseCss.styles == null) {
157+
futures.add(async.Future
158+
.wait(baseCss.urls.map(_urlToStyle))
159+
.then((List<dom.StyleElement> cssList) {
160+
baseCss.styles = cssList;
161+
_insertCss(cssList, shadowRoot, shadowRoot.firstChild);
162+
}));
163+
} else {
164+
_insertCss(baseCss.styles, shadowRoot, shadowRoot.firstChild);
165+
}
166+
}
135167

136-
async.Future<Iterable<dom.StyleElement>> cssFuture;
137-
if (_component.useNgBaseCss == true) {
138-
cssFuture = async.Future.wait([async.Future.wait(baseCss.urls.map(_styleFuture)), _styleElementsFuture]).then((twoLists) {
139-
assert(twoLists.length == 2);return []
140-
..addAll(twoLists[0])
141-
..addAll(twoLists[1]);
142-
});
143-
} else {
144-
cssFuture = _styleElementsFuture;
168+
if (_styleElementsFuture != null) {
169+
if (_styleElements == null) {
170+
futures.add(_styleElementsFuture .then((List<dom.StyleElement> styles) =>
171+
_insertCss(styles, shadowRoot, firstViewNode)));
172+
} else {
173+
_insertCss(_styleElements, shadowRoot);
174+
}
145175
}
146176

147-
ComponentDirectiveInjector shadowInjector;
148177

149-
TemplateLoader templateLoader = new TemplateLoader(cssFuture.then((Iterable<dom.StyleElement> cssList) {
150-
cssList.where((styleElement) => styleElement != null).forEach((styleElement) {
151-
shadowDom.append(styleElement.clone(true));
152-
});
153-
if (_viewFuture != null) {
154-
return _viewFuture.then((ViewFactory viewFactory) {
155-
if (shadowScope.isAttached) {
156-
shadowDom.nodes.addAll(viewFactory.call(shadowInjector.scope, shadowInjector).nodes);
157-
}
158-
return shadowDom;
159-
});
178+
if (_shadowViewFactoryFuture != null) {
179+
if (_shadowViewFactory == null) {
180+
futures.add(_shadowViewFactoryFuture.then((ViewFactory viewFactory) =>
181+
firstViewNode = _insertView(viewFactory, shadowRoot, shadowScope, shadowInjector)));
182+
} else {
183+
_insertView(_shadowViewFactory, shadowRoot, shadowScope, shadowInjector);
160184
}
161-
return shadowDom;
162-
}));
163-
164-
var probe;
165-
var eventHandler = new ShadowRootEventHandler(
166-
shadowDom, injector.getByKey(EXPANDO_KEY), injector.getByKey(EXCEPTION_HANDLER_KEY));
167-
shadowInjector = new ComponentDirectiveInjector(injector, _injector, eventHandler, shadowScope,
168-
templateLoader, shadowDom, null, view);
169-
shadowInjector.bindByKey(_ref.typeKey, _ref.factory, _ref.paramKeys, _ref.annotation.visibility);
185+
}
170186

171187
if (_componentFactory.config.elementProbeEnabled) {
172-
probe = _componentFactory.expando[shadowDom] = shadowInjector.elementProbe;
173-
shadowScope.on(ScopeEvent.DESTROY).listen((ScopeEvent) => _componentFactory.expando[shadowDom] = null);
188+
ElementProbe probe = _componentFactory.expando[shadowRoot] = shadowInjector.elementProbe;
189+
shadowScope.on(ScopeEvent.DESTROY).listen((ScopeEvent) => _componentFactory.expando[shadowRoot] = null);
174190
}
175191

176192
var controller = shadowInjector.getByKey(_ref.typeKey);
@@ -184,6 +200,36 @@ class BoundShadowDomComponentFactory implements BoundComponentFactory {
184200
}
185201
};
186202
}
203+
204+
_insertCss(List<dom.StyleElement> cssList,
205+
dom.ShadowRoot shadowRoot,
206+
[dom.Node insertBefore = null]) {
207+
var s = traceEnter(View_styles);
208+
for(int i = 0; i < cssList.length; i++) {
209+
var styleElement = cssList[i];
210+
if (styleElement != null) {
211+
shadowRoot.insertBefore(styleElement.clone(true), insertBefore);
212+
}
213+
}
214+
traceLeave(s);
215+
}
216+
217+
dom.Node _insertView(ViewFactory viewFactory,
218+
dom.ShadowRoot shadowRoot,
219+
Scope shadowScope,
220+
ComponentDirectiveInjector shadowInjector) {
221+
dom.Node first = null;
222+
if (shadowScope.isAttached) {
223+
View shadowView = viewFactory.call(shadowScope, shadowInjector);
224+
List<dom.Node> shadowViewNodes = shadowView.nodes;
225+
for (var j = 0; j < shadowViewNodes.length; j++) {
226+
var node = shadowViewNodes[j];
227+
if (j == 0) first = node;
228+
shadowRoot.append(node);
229+
}
230+
}
231+
return first;
232+
}
187233
}
188234

189235
class _ComponentAssetKey {

lib/core_dom/transcluding_component_factory.dart

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,18 @@ class BoundTranscludingComponentFactory implements BoundComponentFactory {
2020
final Injector _injector;
2121

2222
Component get _component => _ref.annotation as Component;
23-
async.Future<ViewFactory> _viewFuture;
23+
async.Future<ViewFactory> _viewFactoryFuture;
24+
ViewFactory _viewFactory;
2425

2526
BoundTranscludingComponentFactory(this._f, this._ref, this._directives, this._injector) {
26-
_viewFuture = BoundComponentFactory._viewFuture(
27-
_component,
28-
_f.viewCache,
29-
_directives);
27+
_viewFactoryFuture = BoundComponentFactory._viewFactoryFuture(_component, _f.viewCache, _directives);
28+
if (_viewFactoryFuture != null) {
29+
_viewFactoryFuture.then((viewFactory) => _viewFactory = viewFactory);
30+
}
3031
}
3132

3233
List<Key> get callArgs => _CALL_ARGS;
33-
static var _CALL_ARGS = [ DIRECTIVE_INJECTOR_KEY, SCOPE_KEY, VIEW_KEY,
34+
static var _CALL_ARGS = [ DIRECTIVE_INJECTOR_KEY, SCOPE_KEY,
3435
VIEW_CACHE_KEY, HTTP_KEY, TEMPLATE_CACHE_KEY,
3536
DIRECTIVE_MAP_KEY, NG_BASE_CSS_KEY, EVENT_HANDLER_KEY];
3637
Function call(dom.Node node) {
@@ -39,55 +40,42 @@ class BoundTranscludingComponentFactory implements BoundComponentFactory {
3940
_component.cssUrls.isEmpty);
4041

4142
var element = node as dom.Element;
42-
return (DirectiveInjector injector, Scope scope, View view,
43+
var component = _component;
44+
return (DirectiveInjector injector, Scope scope,
4345
ViewCache viewCache, Http http, TemplateCache templateCache,
4446
DirectiveMap directives, NgBaseCss baseCss, EventHandler eventHandler) {
4547

46-
DirectiveInjector childInjector;
47-
var childInjectorCompleter; // Used if the ViewFuture is available before the childInjector.
48-
49-
var component = _component;
48+
List<async.Future> futures = [];
5049
var lightDom = new LightDom(element, scope);
51-
52-
// Append the component's template as children
53-
var elementFuture;
54-
55-
if (_viewFuture != null) {
56-
elementFuture = _viewFuture.then((ViewFactory viewFactory) {
57-
lightDom.pullNodes();
58-
59-
if (childInjector != null) {
60-
lightDom.shadowDomView = viewFactory.call(childInjector.scope, childInjector);
61-
return element;
62-
} else {
63-
childInjectorCompleter = new async.Completer();
64-
return childInjectorCompleter.future.then((childInjector) {
65-
lightDom.shadowDomView = viewFactory.call(childInjector.scope, childInjector);
66-
return element;
67-
});
68-
}
69-
});
70-
} else {
71-
elementFuture = new async.Future.microtask(() => lightDom.pullNodes());
72-
}
73-
TemplateLoader templateLoader = new TemplateLoader(elementFuture);
74-
50+
TemplateLoader templateLoader = new TemplateLoader(element, futures);
7551
Scope shadowScope = scope.createChild(new HashMap());
76-
77-
childInjector = new ComponentDirectiveInjector(injector, this._injector,
78-
eventHandler, shadowScope, templateLoader, new EmulatedShadowRoot(element), lightDom, view);
79-
52+
DirectiveInjector childInjector = new ComponentDirectiveInjector(
53+
injector, this._injector, eventHandler, shadowScope, templateLoader,
54+
new EmulatedShadowRoot(element), lightDom);
8055
childInjector.bindByKey(_ref.typeKey, _ref.factory, _ref.paramKeys, _ref.annotation.visibility);
8156

82-
if (childInjectorCompleter != null) {
83-
childInjectorCompleter.complete(childInjector);
84-
}
85-
8657
var controller = childInjector.getByKey(_ref.typeKey);
8758
shadowScope.context[component.publishAs] = controller;
8859
if (controller is ScopeAware) controller.scope = shadowScope;
8960
BoundComponentFactory._setupOnShadowDomAttach(controller, templateLoader, shadowScope);
61+
62+
if (_viewFactoryFuture != null && _viewFactory == null) {
63+
futures.add(_viewFactoryFuture.then((ViewFactory viewFactory) =>
64+
_insert(viewFactory, element, childInjector, lightDom)));
65+
} else {
66+
scope.rootScope.runAsync(() {
67+
_insert(_viewFactory, element, childInjector, lightDom);
68+
});
69+
}
9070
return controller;
9171
};
9272
}
73+
74+
_insert(ViewFactory viewFactory, dom.Element element, DirectiveInjector childInjector,
75+
LightDom lightDom) {
76+
lightDom.pullNodes();
77+
if (viewFactory != null) {
78+
lightDom.shadowDomView = viewFactory.call(childInjector.scope, childInjector);
79+
}
80+
}
9381
}

lib/core_dom/view_factory.dart

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,12 @@ class ViewFactory implements Function {
8686
}
8787
elementInjectors[elementBinderIndex] = elementInjector;
8888

89-
if (tagged.textBinders != null) {
90-
for (var k = 0; k < tagged.textBinders.length; k++) {
91-
TaggedTextBinder taggedText = tagged.textBinders[k];
92-
var childNode = boundNode.childNodes[taggedText.offsetIndex];
89+
var textBinders = tagged.textBinders;
90+
if (textBinders != null && textBinders.length > 0) {
91+
var childNodes = boundNode.childNodes;
92+
for (var k = 0; k < textBinders.length; k++) {
93+
TaggedTextBinder taggedText = textBinders[k];
94+
var childNode = childNodes[taggedText.offsetIndex];
9395
taggedText.binder.bind(view, scope, elementInjector, childNode);
9496
}
9597
}
@@ -104,15 +106,6 @@ class ViewFactory implements Function {
104106
dom.Node node = nodeList[i];
105107
NodeLinkingInfo linkingInfo = nodeLinkingInfos[i];
106108

107-
// if node isn't attached to the DOM, create a parent for it.
108-
var parentNode = node.parentNode;
109-
var fakeParent = false;
110-
if (parentNode == null) {
111-
fakeParent = true;
112-
parentNode = new dom.DivElement();
113-
parentNode.append(node);
114-
}
115-
116109
if (linkingInfo.isElement) {
117110
if (linkingInfo.containsNgBinding) {
118111
var tagged = elementBinders[elementBinderIndex];
@@ -138,11 +131,6 @@ class ViewFactory implements Function {
138131
}
139132
elementBinderIndex++;
140133
}
141-
142-
if (fakeParent) {
143-
// extract the node from the parentNode.
144-
nodeList[i] = parentNode.nodes[0];
145-
}
146134
}
147135
return view;
148136
}

lib/core_dom/web_platform.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class WebPlatform {
4242
//
4343
// TODO Remove the try-catch once https://github.com/angular/angular.dart/issues/1189 is fixed.
4444
try {
45-
root.querySelectorAll("*").forEach((n) => n.attributes[selector] = "");
45+
root.querySelectorAll("*").forEach((dom.Element n) => n.setAttribute(selector, ""));
4646
} catch (e, s) {
4747
print("WARNING: Failed to set up Shadow DOM shim for $selector.\n$e\n$s");
4848
}
@@ -69,6 +69,7 @@ class PlatformViewCache implements ViewCache {
6969
if (selector != null && selector != "" && platform.shadowDomShimRequired) {
7070
// By adding a comment with the tag name we ensure the template html is unique per selector
7171
// name when used as a key in the view factory cache.
72+
//TODO(misko): This will always be miss, since we never put it in cache under such key.
7273
viewFactory = viewFactoryCache.get("<!-- Shimmed template for: <$selector> -->$html");
7374
} else {
7475
viewFactory = viewFactoryCache.get(html);

0 commit comments

Comments
 (0)