Merge pull request #65 from gerich-home/fixing-diamond-perf

Fixing diamond perf
gerich-home · Apr 5, 2016 · 62f3eed · 62f3eed
2 parents 93acfdf + e1f37d4
commit 62f3eed
Show file tree

Hide file tree

Showing 9 changed files with 145 additions and 59 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -8,7 +8,7 @@
 			"program": "${workspaceRoot}/node_modules/gulp/bin/gulp.js",
 			"stopOnEntry": false,
 			"args": [
-                "unit-tests-only"
+                "unit-tests"
             ],
 			"cwd": "${workspaceRoot}",
 			"preLaunchTask": null,

diff --git a/.vscode/tasks.json b/.vscode/tasks.json
@@ -18,9 +18,13 @@
             ]
         },
         {
-            "taskName": "unit-tests-only",
+            "taskName": "unit-tests",
             "args": [],
             "isTestCommand": true
+        },
+        {
+            "taskName": "performance-tests",
+            "args": []
         }
     ]
 }
diff --git a/performance-tests/scenarios/computedDiamondUpdate.js b/performance-tests/scenarios/computedDiamondUpdate.js
@@ -49,6 +49,10 @@ module.exports = function(updatesCount, dependenciesCount) {
             var e = ko.pureComputed(function() {
                 return d() % 2 == 0 ? b() : c();
             });
+
+            var s = e.subscribe(function(){
+
+            });
 
             return d;
         }

diff --git a/specs/realWorldUseCases.js b/specs/realWorldUseCases.js
@@ -164,4 +164,46 @@ describe('usage of the library in the real world scenarios', function () {
 			});
 		});
 	});
+
+	context('diamond dependencies', function () {
+		it('should work', function() {
+            var dependenciesCount = 3;
+            var updatesCount = 3;
+
+            var x = [];
+            for (var j = 0; j < dependenciesCount; j++) {
+                x.push(itDepends.value(j));
+            }
+
+            var a = itDepends.computed(function() {
+                var z = 0;
+                for (var j = 0; j < dependenciesCount; j++) {
+                    z += x[j]();
+                }
+                return z;
+            });
+
+            var b = itDepends.computed(function() {
+                return a();
+            });
+
+            var c = itDepends.computed(function() {
+                return a();
+            });
+
+            var d = itDepends.value(-1);
+
+            var e = itDepends.computed(function() {
+                return d() % 2 == 0 ? b() : c();
+            });
+
+            var s = e.onChange(function(){
+
+            });
+
+            for (var j = 0; j < updatesCount; j++) {
+                d.write(j);
+            }
+		});
+    });
 });
diff --git a/src/bulkChange.ts b/src/bulkChange.ts
@@ -1,65 +1,67 @@
 'use strict';
 
 import { IHasValue } from './subscriptionList';
+import { doChange } from './change';
 
 interface IChange<T> {
     value: IHasValue<T>;
-    notify(): void;
+    oldValue: T;
+    newValue?: T;
+    notify(from: T, to: T): void;
 }
 
-interface IChanges {
-    [id: number]: IChange<any>;
-    ids: number[];
+interface IHasChanges {
+    [id: number]: boolean;
 }
 
 var bulkLevels: number = 0;
-var changes: IChanges;
+var changes: IChange<any>[];
+var hasChange: IHasChanges;
 
 export function valueChanged<T>(id: number, value: IHasValue<T>, oldValue: T, notify: (from: T, to: T) => void): void {
     if (bulkLevels === 0) {
         notify(oldValue, value());
-    } else if (!changes[id]) {
-        changes.ids.push(id);
+    } else if (hasChange[id] === undefined) {
+        changes.push({
+            notify: notify,
+            value: value,
+            oldValue: oldValue
+        });
 
-        changes[id] = {
-            notify: function(): void {
-                var newValue = value();
-                if (oldValue !== newValue) {
-                    notify(oldValue, newValue);
-                }
-            },
-            value: value
-        };
+        hasChange[id] = true;
     }
 }
 
 export default function(changeAction: () => void): void {
     var isFirstBulk = bulkLevels === 0;
+
     if (isFirstBulk) {
-        changes = {
-            ids: []
-        };
+        changes = [];
+        hasChange = {};
     }
 
     bulkLevels++;
 
     try {
-        changeAction();
+        doChange(changeAction);
     } finally {
         if (isFirstBulk) {
-            for (var id of changes.ids) {
-                changes[id].value();
+            for (var change of changes) {
+                change.newValue = change.value();
             }
         }
 
         bulkLevels--;
 
         if (isFirstBulk) {
-            for (var id of changes.ids) {
-                changes[id].notify();
+            for (var change of changes) {
+                if (change.oldValue !== change.newValue) {
+                    change.notify(change.oldValue, change.newValue);
+                }
             }
 
             changes = undefined;
+            hasChange = undefined;
         }
     }
 }
diff --git a/src/change.ts b/src/change.ts
@@ -0,0 +1,36 @@
+'use strict';
+
+var changeLevels: number = 0;
+var actions: (() => void)[];
+
+export function onChangeFinished(action: () => void): void {
+    if (changeLevels === 0) {
+        action();
+    } else {
+        actions.push(action);
+    }
+}
+
+export function doChange(changeAction: () => void): void {
+    var isFirstChange = changeLevels === 0;
+
+    if (isFirstChange) {
+        actions = [];
+    }
+
+    changeLevels++;
+
+    try {
+        changeAction();
+    } finally {
+        changeLevels--;
+
+        if (isFirstChange) {
+            for (var action of actions) {
+                action();
+            }
+
+            actions = undefined;
+        }
+    }
+}
diff --git a/src/computed.ts b/src/computed.ts
@@ -3,6 +3,7 @@
 import * as tracking from './tracking';
 import { default as subscriptionList, ISubscription, ISubscriptions, IHasValue } from './subscriptionList';
 import { valueChanged } from './bulkChange';
+import { onChangeFinished } from './change';
 
 export interface ICalculator<T> {
     (params: any[]): T;
@@ -47,29 +48,28 @@ export default function<T>(calculator: ICalculator<T>, args: any[], writeCallbac
     var self: IComputed<T>;
 
     var atLeastOneDependencyChanged = function(): boolean {
-        for (var i = 0; i < dependencies.length; i++) {
-            var dependency = dependencies[i];
-
-            if (dependency.observableValue() !== dependency.capturedValue) {
-                return true;
-            }
-        }
+        return tracking
+            .trackingWith(undefined)
+            .execute(function(): boolean {
+                for (var dependency of dependencies) {
+                    if (dependency.observableValue() !== dependency.capturedValue) {
+                        return true;
+                    }
+                }
 
-        return false;
+                return false;
+            });
     };
 
     var unsubscribeDependencies = function(): void {
-        for (var i = 0; i < dependencies.length; i++) {
-            var dependency = dependencies[i];
+        for (var dependency of dependencies) {
             dependency.subscription.disable();
             dependency.subscription = undefined;
         }
     };
 
     var subscribeDependencies = function(): void {
-        for (var i = 0; i < dependencies.length; i++) {
-            var dependency = dependencies[i];
-
+        for (var dependency of dependencies) {
             dependency.subscription = dependency.observableValue.onChange(self);
         }
     };
@@ -94,7 +94,7 @@ export default function<T>(calculator: ICalculator<T>, args: any[], writeCallbac
 
                 tracking
                     .trackingWith(function(dependencyId: number, observableValue: any, capturedValue: any): void {
-                        if (dependenciesById[dependencyId]) {
+                        if (dependenciesById[dependencyId] !== undefined) {
                             return;
                         }
 
@@ -117,36 +117,31 @@ export default function<T>(calculator: ICalculator<T>, args: any[], writeCallbac
 
                 if (subscriptionsActive) {
                     if (oldDependencies) {
-                        for (var i = 0; i < oldDependencies.length; i++) {
-                            var oldDependency = oldDependencies[i];
+                        for (var oldDependency of oldDependencies) {
                             var newDependency = dependenciesById[oldDependency.dependencyId];
 
-                            if (newDependency) {
+                            if (newDependency !== undefined) {
                                 newDependency.subscription = oldDependency.subscription;
                             }
                         }
                     }
 
-                    for (var i = 0; i < dependencies.length; i++) {
-                        var dependency = dependencies[i];
+                    for (var dependency of dependencies) {
                         dependency.subscription = dependency.subscription || dependency.observableValue.onChange(self);
                     }
 
                     if (oldDependencies) {
-                        for (var i = 0; i < oldDependencies.length; i++) {
-                            var oldDependency = oldDependencies[i];
-                            var newDependency = dependenciesById[oldDependency.dependencyId];
+                        onChangeFinished(() => {
+                            for (var oldDependency of oldDependencies) {
+                                var newDependency = dependenciesById[oldDependency.dependencyId];
 
-                            if (!newDependency) {
-                                oldDependency.subscription.disable();
+                                if (newDependency === undefined) {
+                                    oldDependency.subscription.disable();
+                                }
                             }
-                        }
-
-                        oldDependencies = undefined;
+                        });
                     }
 
-                    dependenciesById = undefined;
-
                     if (oldValue !== currentValue) {
                         valueChanged(id, self, oldValue, notifySubscribers);
                     }

diff --git a/src/tracking.ts b/src/tracking.ts
@@ -17,17 +17,17 @@ export function takeNextObservableId(): number {
 }
 
 export interface ITrackerExecution {
-    execute(action: () => void): void;
+    execute<T>(action: () => T): T;
 }
 
 export function trackingWith(tracker: Tracker): ITrackerExecution {
     return {
-        execute: function(action: () => void): void {
+        execute: function<T>(action: () => T): T {
             trackers.push(activeTracker);
             activeTracker = tracker;
 
             try {
-                action();
+                return action();
             } finally {
                 activeTracker = trackers.pop();
             }

diff --git a/src/value.ts b/src/value.ts
@@ -2,6 +2,7 @@
 
 import changeNotification from './changeNotification';
 import { valueChanged } from './bulkChange';
+import { doChange } from './change';
 import * as tracking from './tracking';
 import { default as subscriptionList, ISubscription, IChangeHandler, ISubscriptions, IHasValue } from './subscriptionList';
 
@@ -36,7 +37,9 @@ export default function<T>(initialValue: T): IValue<T> {
         currentValue = newValue;
         tracking.lastWriteVersion++;
 
-        valueChanged(id, self, oldValue, notifySubscribers);
+        doChange(() => {
+            valueChanged(id, self, oldValue, notifySubscribers);
+        });
     };
 
     self.onChange = function(handler: IChangeHandler<T>): ISubscription {