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

Exp coalescing #1328

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 8 additions & 30 deletions lib/change_detection/watch_group.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
final ChangeDetectorGroup<_Handler> _changeDetector;
/** A cache for sharing sub expression watching. Watching `a` and `a.b` will
* watch `a` only once. */
final Map<String, WatchRecord<_Handler>> _cache;
final RootWatchGroup _rootGroup;

/// STATS: Number of field watchers which are in use.
Expand Down Expand Up @@ -108,7 +107,7 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
WatchGroup _prevWatchGroup, _nextWatchGroup;

WatchGroup._child(_parentWatchGroup, this._changeDetector, this.context,
this._cache, this._rootGroup)
this._rootGroup)
: _parentWatchGroup = _parentWatchGroup,
id = '${_parentWatchGroup.id}.${_parentWatchGroup._nextChildId++}'
{
Expand All @@ -119,8 +118,7 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
WatchGroup._root(this._changeDetector, this.context)
: id = '',
_rootGroup = null,
_parentWatchGroup = null,
_cache = new HashMap<String, WatchRecord<_Handler>>()
_parentWatchGroup = null
{
_marker.watchGrp = this;
_evalWatchTail = _evalWatchHead = _marker;
Expand All @@ -139,10 +137,7 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
}

Watch watch(AST expression, ReactionFn reactionFn) {
WatchRecord<_Handler> watchRecord = _cache[expression.expression];
if (watchRecord == null) {
_cache[expression.expression] = watchRecord = expression.setupWatch(this);
}
WatchRecord<_Handler> watchRecord = expression.setupWatch(this);
return watchRecord.handler.addReactionFn(reactionFn);
}

Expand All @@ -161,10 +156,7 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
_fieldCost++;
fieldHandler.watchRecord = watchRecord;

WatchRecord<_Handler> lhsWR = _cache[lhs.expression];
if (lhsWR == null) {
lhsWR = _cache[lhs.expression] = lhs.setupWatch(this);
}
WatchRecord<_Handler> lhsWR = lhs.setupWatch(this);

// We set a field forwarding handler on LHS. This will allow the change
// objects to propagate to the current WatchRecord.
Expand All @@ -180,10 +172,7 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
var watchRecord = _changeDetector.watch(null, null, collectionHandler);
_collectionCost++;
collectionHandler.watchRecord = watchRecord;
WatchRecord<_Handler> astWR = _cache[ast.expression];
if (astWR == null) {
astWR = _cache[ast.expression] = ast.setupWatch(this);
}
WatchRecord<_Handler> astWR = ast.setupWatch(this);

// We set a field forwarding handler on LHS. This will allow the change
// objects to propagate to the current WatchRecord.
Expand Down Expand Up @@ -234,32 +223,23 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
invokeHandler.watchRecord = evalWatchRecord;

if (lhsAST != null) {
var lhsWR = _cache[lhsAST.expression];
if (lhsWR == null) {
lhsWR = _cache[lhsAST.expression] = lhsAST.setupWatch(this);
}
var lhsWR = lhsAST.setupWatch(this);
lhsWR.handler.addForwardHandler(invokeHandler);
invokeHandler.acceptValue(lhsWR.currentValue);
}

// Convert the args from AST to WatchRecords
for (var i = 0; i < argsAST.length; i++) {
var ast = argsAST[i];
WatchRecord<_Handler> record = _cache[ast.expression];
if (record == null) {
record = _cache[ast.expression] = ast.setupWatch(this);
}
WatchRecord<_Handler> record = ast.setupWatch(this);
_ArgHandler handler = new _PositionalArgHandler(this, evalWatchRecord, i);
_ArgHandlerList._add(invokeHandler, handler);
record.handler.addForwardHandler(handler);
handler.acceptValue(record.currentValue);
}

namedArgsAST.forEach((Symbol name, AST ast) {
WatchRecord<_Handler> record = _cache[ast.expression];
if (record == null) {
record = _cache[ast.expression] = ast.setupWatch(this);
}
WatchRecord<_Handler> record = ast.setupWatch(this);
_ArgHandler handler = new _NamedArgHandler(this, evalWatchRecord, name);
_ArgHandlerList._add(invokeHandler, handler);
record.handler.addForwardHandler(handler);
Expand Down Expand Up @@ -301,7 +281,6 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
this,
_changeDetector.newGroup(),
context == null ? this.context : context,
new HashMap<String, WatchRecord<_Handler>>(),
_rootGroup == null ? this : _rootGroup);
_WatchGroupList._add(this, childGroup);
var marker = childGroup._marker;
Expand Down Expand Up @@ -581,7 +560,6 @@ abstract class _Handler implements _LinkedList, _LinkedListItem, _WatchList {
_releaseWatch();
// Remove ourselves from cache, or else new registrations will go to us,
// but we are dead
watchGrp._cache.remove(expression);

if (forwardingHandler != null) {
// TODO(misko): why do we need this check?
Expand Down
9 changes: 8 additions & 1 deletion lib/core_dom/common.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
part of angular.core.dom_internal;

List<dom.Node> cloneElements(elements) => elements.map((el) => el.clone(true)).toList();
List<dom.Node> cloneElements(List<dom.Node> elements) {
int length = elements.length;
var clones = new List<dom.Node>(length);
for(var i=0; i < length; i++) {
clones[i] = elements[i].clone(true);
}
return clones;
}

class MappingParts {
final String attrName;
Expand Down
22 changes: 8 additions & 14 deletions lib/core_dom/node_cursor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ part of angular.core.dom_internal;

class NodeCursor {
final stack = [];
List<dom.Node> elements;
List<dom.Node> elements; // may be a fixed length list.
int index = 0;

NodeCursor(this.elements);
Expand All @@ -29,21 +29,15 @@ class NodeCursor {
index = stack.removeLast();
}

void insertAnchorBefore(String name) {
var parent = current.parentNode;
var anchor = new dom.Comment('ANCHOR: $name');
elements.insert(index++, anchor);
if (parent != null) parent.insertBefore(anchor, current);
}

NodeCursor replaceWithAnchor(String name) {
insertAnchorBefore(name);
var childCursor = remove();
index--;
return childCursor;
var element = current;
var parent = element.parentNode;
var anchor = new dom.Comment('ANCHOR: $name');
if (parent != null) parent.insertBefore(anchor, element);
element.remove();
elements[index] = anchor;
return new NodeCursor([element]);
}

NodeCursor remove() => new NodeCursor([elements.removeAt(index)..remove()]);

String toString() => "[NodeCursor: $elements $index]";
}
2 changes: 1 addition & 1 deletion lib/directive/ng_repeat.dart
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ class _Row {
View view;
dom.Element startNode;
dom.Element endNode;
List<dom.Element> nodes;
List<dom.Node> nodes;

_Row(this.id);
}
12 changes: 6 additions & 6 deletions test/change_detection/watch_group_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ void main() {
var watch = watchGrp.watch(parse('user'), (v, p) => logger(v));
var watchFirst = watchGrp.watch(parse('user.first'), (v, p) => logger(v));
var watchLast = watchGrp.watch(parse('user.last'), (v, p) => logger(v));
expect(watchGrp.fieldCost).toEqual(3);
expect(watchGrp.fieldCost).toEqual(5);

watchGrp.detectChanges();
expect(logger).toEqual([user1, 'misko', 'hevery']);
Expand All @@ -285,7 +285,7 @@ void main() {


watch.remove();
expect(watchGrp.fieldCost).toEqual(3);
expect(watchGrp.fieldCost).toEqual(4);

watchFirst.remove();
expect(watchGrp.fieldCost).toEqual(2);
Expand Down Expand Up @@ -863,8 +863,8 @@ void main() {
expect(watchGrp.detectChanges()).toEqual(6);
// expect(logger).toEqual(['0a', '0A', '1a', '1A', '2A', '1b']);
expect(logger).toEqual(['0a', '1a', '1b', '0A', '1A', '2A']); // we go by registration order
expect(watchGrp.fieldCost).toEqual(1);
expect(watchGrp.totalFieldCost).toEqual(4);
expect(watchGrp.fieldCost).toEqual(2);
expect(watchGrp.totalFieldCost).toEqual(6);
logger.clear();

context['a'] = 1;
Expand All @@ -876,8 +876,8 @@ void main() {
child1a.remove(); // should also remove child2
expect(watchGrp.detectChanges()).toEqual(3);
expect(logger).toEqual(['0a', '0A', '1b']);
expect(watchGrp.fieldCost).toEqual(1);
expect(watchGrp.totalFieldCost).toEqual(2);
expect(watchGrp.fieldCost).toEqual(2);
expect(watchGrp.totalFieldCost).toEqual(3);
});

it('should remove all method watches in group and group\'s children', () {
Expand Down