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

Commit 5424b4d

Browse files
mheveryrkirov
authored andcommitted
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 cb9a9d7 commit 5424b4d

File tree

9 files changed

+173
-115
lines changed

9 files changed

+173
-115
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));
@@ -67,20 +67,27 @@ class BoundShadowDomComponentFactory implements BoundComponentFactory {
6767
Component get _component => _ref.annotation as Component;
6868

6969
String _tag;
70-
async.Future<Iterable<dom.StyleElement>> _styleElementsFuture;
71-
async.Future<ViewFactory> _viewFuture;
70+
async.Future<List<dom.StyleElement>> _styleElementsFuture;
71+
List<dom.StyleElement> _styleElements;
72+
async.Future<ViewFactory> _shadowViewFactoryFuture;
73+
ViewFactory _shadowViewFactory;
7274

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

77-
_viewFuture = BoundComponentFactory._viewFuture(
80+
_shadowViewFactoryFuture = BoundComponentFactory._viewFactoryFuture(
7881
_component,
82+
// TODO(misko): Why do we create a new one per Component. This kind of defeats the caching.
7983
new PlatformViewCache(_componentFactory.viewCache, _tag, _componentFactory.platform),
8084
_directives);
85+
if (_shadowViewFactoryFuture != null) {
86+
_shadowViewFactoryFuture.then((viewFactory) => _shadowViewFactory = viewFactory);
87+
}
8188
}
8289

83-
async.Future<dom.StyleElement> _styleFuture(cssUrl) {
90+
async.Future<dom.StyleElement> _urlToStyle(cssUrl) {
8491
Http http = _componentFactory.http;
8592
TemplateCache templateCache = _componentFactory.templateCache;
8693
WebPlatform platform = _componentFactory.platform;
@@ -109,7 +116,7 @@ class BoundShadowDomComponentFactory implements BoundComponentFactory {
109116

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

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

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

148-
ComponentDirectiveInjector shadowInjector;
149178

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

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

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

190236
class _ComponentAssetKey {

lib/core_dom/transcluding_component_factory.dart

Lines changed: 32 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,14 @@ class BoundTranscludingComponentFactory implements BoundComponentFactory {
8888
final Injector _injector;
8989

9090
Component get _component => _ref.annotation as Component;
91-
async.Future<ViewFactory> _viewFuture;
91+
async.Future<ViewFactory> _viewFactoryFuture;
92+
ViewFactory _viewFactory;
9293

9394
BoundTranscludingComponentFactory(this._f, this._ref, this._directives, this._injector) {
94-
_viewFuture = BoundComponentFactory._viewFuture(
95-
_component,
96-
_f.viewCache,
97-
_directives);
95+
_viewFactoryFuture = BoundComponentFactory._viewFactoryFuture(_component, _f.viewCache, _directives);
96+
if (_viewFactoryFuture != null) {
97+
_viewFactoryFuture.then((viewFactory) => _viewFactory = viewFactory);
98+
}
9899
}
99100

100101
List<Key> get callArgs => _CALL_ARGS;
@@ -107,56 +108,45 @@ class BoundTranscludingComponentFactory implements BoundComponentFactory {
107108
_component.cssUrls.isEmpty);
108109

109110
var element = node as dom.Element;
111+
var component = _component;
110112
return (DirectiveInjector injector, Scope scope,
111113
ViewCache viewCache, Http http, TemplateCache templateCache,
112114
DirectiveMap directives, NgBaseCss baseCss, EventHandler eventHandler) {
113115

114-
DirectiveInjector childInjector;
115-
var childInjectorCompleter; // Used if the ViewFuture is available before the childInjector.
116-
117-
var component = _component;
116+
List<async.Future> futures = [];
118117
var contentPort = new ContentPort(element);
119-
120-
// Append the component's template as children
121-
var elementFuture;
122-
123-
if (_viewFuture != null) {
124-
elementFuture = _viewFuture.then((ViewFactory viewFactory) {
125-
contentPort.pullNodes();
126-
if (childInjector != null) {
127-
element.nodes.addAll(
128-
viewFactory.call(childInjector.scope, childInjector).nodes);
129-
return element;
130-
} else {
131-
childInjectorCompleter = new async.Completer();
132-
return childInjectorCompleter.future.then((childInjector) {
133-
element.nodes.addAll(
134-
viewFactory.call(childInjector.scope, childInjector).nodes);
135-
return element;
136-
});
137-
}
138-
});
139-
} else {
140-
elementFuture = new async.Future.microtask(() => contentPort.pullNodes());
141-
}
142-
TemplateLoader templateLoader = new TemplateLoader(elementFuture);
143-
118+
TemplateLoader templateLoader = new TemplateLoader(element, futures);
144119
Scope shadowScope = scope.createChild(new HashMap());
145-
146-
childInjector = new ComponentDirectiveInjector(injector, this._injector,
147-
eventHandler, shadowScope, templateLoader, new ShadowlessShadowRoot(element),
148-
contentPort);
120+
DirectiveInjector childInjector = new ComponentDirectiveInjector(
121+
injector, this._injector, eventHandler, shadowScope, templateLoader,
122+
new ShadowlessShadowRoot(element), contentPort);
149123
childInjector.bindByKey(_ref.typeKey, _ref.factory, _ref.paramKeys, _ref.annotation.visibility);
150124

151-
if (childInjectorCompleter != null) {
152-
childInjectorCompleter.complete(childInjector);
153-
}
154-
155125
var controller = childInjector.getByKey(_ref.typeKey);
156126
shadowScope.context[component.publishAs] = controller;
157127
if (controller is ScopeAware) controller.scope = shadowScope;
158128
BoundComponentFactory._setupOnShadowDomAttach(controller, templateLoader, shadowScope);
129+
130+
if (_viewFactoryFuture != null && _viewFactory == null) {
131+
futures.add(_viewFactoryFuture.then((ViewFactory viewFactory) =>
132+
_insert(viewFactory, element, childInjector, contentPort)));
133+
} else {
134+
scope.rootScope.runAsync(() {
135+
_insert(_viewFactory, element, childInjector, contentPort);
136+
});
137+
}
159138
return controller;
160139
};
161140
}
141+
142+
_insert(ViewFactory viewFactory, dom.Element element, DirectiveInjector childInjector,
143+
ContentPort contentPort) {
144+
contentPort.pullNodes();
145+
if (viewFactory != null) {
146+
var viewNodes = viewFactory.call(childInjector.scope, childInjector).nodes;
147+
for(var i = 0; i < viewNodes.length; i++) {
148+
element.append(viewNodes[i]);
149+
}
150+
}
151+
}
162152
}

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)