Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.
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
140 changes: 140 additions & 0 deletions lib/change_detection/ast.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
part of angular.watch_group;


/**
* RULES:
* - ASTs are reusable. Don't store scope/instance refs there
* - Parent knows about children, not the other way around.
*/
abstract class AST {
static final String _CONTEXT = '#';
final String expression;
AST(this.expression) { assert(expression!=null); }
WatchRecord<_Handler> setupWatch(WatchGroup watchGroup);
toString() => expression;
}

/**
* SYNTAX: _context_
*
* This represent the initial _context_ for evaluation.
*/
class ContextReferenceAST extends AST {
ContextReferenceAST(): super(AST._CONTEXT);
WatchRecord<_Handler> setupWatch(WatchGroup watchGroup)
=> new _ConstantWatchRecord(watchGroup, expression, watchGroup.context);
}

/**
* SYNTAX: _context_
*
* This represent the initial _context_ for evaluation.
*/
class ConstantAST extends AST {
final constant;

ConstantAST(dynamic constant):
super('$constant'),
constant = constant;

WatchRecord<_Handler> setupWatch(WatchGroup watchGroup)
=> new _ConstantWatchRecord(watchGroup, expression, constant);
}

/**
* SYNTAX: lhs.name.
*
* This is the '.' dot operator.
*/
class FieldReadAST extends AST {
AST lhs;
final String name;

FieldReadAST(lhs, name)
: super(lhs.expression == AST._CONTEXT ? name : '$lhs.$name'),
lhs = lhs,
name = name;

WatchRecord<_Handler> setupWatch(WatchGroup watchGroup) =>
watchGroup.addFieldWatch(lhs, name, expression);

}

/**
* SYNTAX: fn(arg0, arg1, ...)
*
* Invoke a pure function. Pure means that the function has no state, and
* therefore it needs to be re-computed only if its args change.
*/
class PureFunctionAST extends AST {
final String name;
final Function fn;
final List<AST> argsAST;

PureFunctionAST(name, this.fn, argsAST)
: super('$name(${_argList(argsAST)})'),
argsAST = argsAST,
name = name;

WatchRecord<_Handler> setupWatch(WatchGroup watchGroup) =>
watchGroup.addFunctionWatch(fn, argsAST, expression);
}

/**
* SYNTAX: lhs.method(arg0, arg1, ...)
*
* Invoke a method on [lhs] object.
*/
class MethodAST extends AST {
final AST lhsAST;
final String name;
final List<AST> argsAST;

MethodAST(lhsAST, name, argsAST)
: super('$lhsAST.$name(${_argList(argsAST)})'),
lhsAST = lhsAST,
name = name,
argsAST = argsAST;

WatchRecord<_Handler> setupWatch(WatchGroup watchGroup) =>
watchGroup.addMethodWatch(lhsAST, name, argsAST, expression);
}


class CollectionAST extends AST {
final AST valueAST;
CollectionAST(valueAST):
super('#collection($valueAST)'),
valueAST = valueAST;

WatchRecord<_Handler> setupWatch(WatchGroup watchGroup) {
return watchGroup.addCollectionWatch(valueAST);
}
}

_argList(List<AST> items) => items.join(', ');

/**
* The name is a bit oxymoron, but it is essentially the NullObject pattern.
*
* This allows children to set a handler on this ChangeRecord and then let it write the initial
* constant value to the forwarding ChangeRecord.
*/
class _ConstantWatchRecord extends WatchRecord<_Handler> {
final currentValue;
final _Handler handler;

_ConstantWatchRecord(WatchGroup watchGroup, String expression, dynamic currentValue):
currentValue = currentValue,
handler = new _ConstantHandler(watchGroup, expression, currentValue);

ChangeRecord<_Handler> check() => null;
void remove() => null;

get field => null;
get previousValue => null;
get object => null;
set object(_) => null;
get nextChange => null;
}

178 changes: 178 additions & 0 deletions lib/change_detection/change_detection.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
library change_detection;

/**
* An interface for [ChangeDetectorGroup] groups related watches together. It
* guarentees that within the group all watches will be reported in the order in
* which they were registered. It also provides an efficient way of removing the
* watch group.
*/
abstract class ChangeDetectorGroup<H> {
/**
* Watch a specific [field] on an [object].
*
* If the [field] is:
* - _name_ - Name of the field to watch. (If the [object] is a Map then
* treat it as a key.)
* - _[]_ - Watch all items in an array.
* - _{}_ - Watch all items in a Map.
* - _._ - Watch the actual object identity.
*
*
* Parameters:
* - [object] to watch.
* - [field] to watch on the [object].
* - [handler] an opaque object passed on to [ChangeRecord].
*/
WatchRecord<H> watch(Object object, String field, H handler);


/** Use to remove all watches in the group in an efficient manner. */
void remove();

/** Create a child [ChangeDetectorGroup] */
ChangeDetectorGroup<H> newGroup();
}

/**
* An interface for [ChangeDetector]. An application can have multiple instances
* of the [ChangeDetector] to be used for checking different application domains.
*
* [ChangeDetector] works by comparing the identity of the objects not by
* calling the `.equals()` method. This is because ChangeDetector needs to have
* predictable performance, and the developer can implement `.equals()` on top
* of identity checks.
*
* - [H] A [ChangeRecord] has associated handler object. The handler object is
* opaque to the [ChangeDetector] but it is meaningful to the code which
* registered the watcher. It can be a data structure, an object, or a function.
* It is up to the developer to attach meaning to it.
*/
abstract class ChangeDetector<H> extends ChangeDetectorGroup<H> {
/**
* This method does the work of collecting the changes and returns them as a
* linked list of [ChangeRecord]s. The [ChangeRecord]s are to be returned in
* the same order as they were registered.
*/
ChangeRecord<H> collectChanges();
}

abstract class Record<H> {
/** The observed object. */
Object get object;

/**
* The field which is being watched:
* - _name_ - Name of the field to watch.
* - _[]_ - Watch all items in an array.
* - _{}_ - Watch all items in a Map.
* - _._ - Watch the actual object identity.
*/
String get field;

/**
* An application provided object which contains the specific logic which
* needs to be applied when the change is detected. The handler is opaque to
* the ChangeDetector and as such can be anything the application desires.
*/
H get handler;

/** Current value of the [field] on the [object] */
get currentValue;
/** Previous value of the [field] on the [object] */
get previousValue;
}

/**
* [WatchRecord] API which allows changing what object is being watched and
* manually triggering the checking.
*/
abstract class WatchRecord<H> extends Record<H> {
/** Set a new object for checking */
set object(value);

/**
* Check to see if the field on the object has changed. Returns [null] if no
* change, or a [ChangeRecord] if the change has been detected.
*/
ChangeRecord<H> check();

void remove();
}

/**
* Provides information about the changes which were detected in objects.
*
* It exposes a `nextChange` method for traversing all of the changes.
*/
abstract class ChangeRecord<H> extends Record<H> {
/** Next [ChangeRecord] */
ChangeRecord<H> get nextChange;
}

/**
* If [ChangeDetector] is watching a collection (an [Iterable]) then the
* [currentValue] of [Record] will contain this object. The object contains a
* summary of changes to the collection since the last execution. The changes
* are reported as a list of [CollectionChangeItem]s which contain the current
* and previous position in the list as well as the item.
*/
abstract class CollectionChangeRecord<K, V> {
/** The underlying iterable object */
Iterable get iterable;

/** A list of [CollectionItem]s which are in the iteration order. */
CollectionItem<K, V> get collectionHead;
/** A list of new [AddedItem]s. */
AddedItem<K, V> get additionsHead;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds strange that AddedItem (singular) is a list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of it as AddedItem is an item. additionsHead points to the first item in that list.

Sounds like it is confusing. Do you have a suggestion for better name?

/** A list of [MovedItem]s. */
MovedItem<K, V> get movesHead;
/** A list of [RemovedItem]s. */
RemovedItem<K, V> get removalsHead;
}

/**
* Each item in collection is wrapped in [CollectionChangeItem], which can track
* the [item]s [currentKey] and [previousKey] location.
*/
abstract class CollectionChangeItem<K, V> {
/** Previous item location in the list or [null] if addition. */
K get previousKey;

/** Current item location in the list or [null] if removal. */
K get currentKey;

/** The item. */
V get item;
}

/**
* Used to create a linked list of collection items.
* These items are always in the iteration order of the collection.
*/
abstract class CollectionItem<K, V> extends CollectionChangeItem<K, V> {
CollectionItem<K, V> get nextCollectionItem;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this benefit from a container class, ie LinkedList<CollectionChangeItem<K, V>>. The container could be reused for all 4 classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, code reuse would be really nice. Unfortunately I can't figure out how do that and not violate some of the design goals.

The goal is that we use the least amount of memory and that there is no GC pressure when the algorithm is running. This is achieved by Item having multiple next/prev pointers. So depending which pointers you follow you will get a list of items, list of changes, list of removals and so on.

Dart's LinkedList implementation means that the Item can only be in one LinkedList. Hence my own implementation of lists.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. A hand-written linked list is superior for this kind of high performance code. At least that's what Ive seen in my own benchmarking.

Random thought: I've heard that generic type args can add some performance overhead, at least on the VM (I guess they do some extra work to track the reified generic). It'd be interesting to see if removing the generic type arguments makes things faster.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be interesting to see if removing the generic type arguments makes things faster.

"Premature optimization is the root of all evil"... I think that the focus should be first on getting the code right & clean first before any obfuscating optim, don't you ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi vic, I completely agree with that principle in general, but this is not premature optimization. The dirty checking code is one of the hottest spots in the system. If there is anything that deserves in depth, pull-out-the-stops crazy optimizations, it is this code :). @mhevery is measuring this code very carefully to get maximum performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the hottest piece of code in the system, and I actually built it from a benchmark out. I first wrote the fastests loop and then kept adding to it until I got this algorithm. So any kinds of ideas on making it faster are very much appreciated.

The code is working and is ready for integration so this is an excellent time for such suggestions. I removed generics, and there was no difference in performance, so I am keeping them in for now.

If you have other do share them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully understand that this code is critical but as of now the implementation is incomplete (ie map not implemented last time I checked).

I think we should at least keep distinct commits while the code is under dev so that it's easier to follow what's going on. Commits are cheap, we can use plenty of them and merge when the code gets ready.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on distinct commits. My plan is to check this in after this round of comments. and then add more commits on top of it.

Yes Maps are not implemented. We don't currently need them, so I am ignoring them for now, but if you want to take a stab at it it would be appreciated.

}

/**
* A linked list of new items added to the collection.
* These items are always in the iteration order of the collection.
*/
abstract class AddedItem<K, V> extends CollectionChangeItem<K, V> {
AddedItem<K, V> get nextAddedItem;
}

/**
* A linked list of moved items in to the collection.
* These items are always in the iteration order of the collection.
*/
abstract class MovedItem<K, V> extends CollectionChangeItem<K, V> {
MovedItem<K, V> get nextMovedItem;
}

/**
* A linked list of removed items in to the collection.
* These items are always in the iteration order of the collection.
*/
abstract class RemovedItem<K, V> extends CollectionChangeItem<K, V> {
RemovedItem<K, V> get nextRemovedItem;
}
Loading