Skip to content

Commit

Permalink
Reduce the stack overhead of observable notifications by eliminating …
Browse files Browse the repository at this point in the history
…four function call levels. Also reduce stack overhead by not using "call" in computed unless needed.
  • Loading branch information
mbest committed Aug 31, 2013
1 parent 44fd8a0 commit 688a567
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
18 changes: 18 additions & 0 deletions spec/dependentObservableBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,4 +390,22 @@ describe('Dependent Observable', function() {
observable(1);
expect(computed()).toEqual(1);
});

// Borrowed from haberman/knockout (see knockout/knockout#359)
it('Should allow long chains without overflowing the stack', function() {
// maximum with previous code (when running this test only): Chrome 28: 1310, IE 10: 2200; FF 23: 103
// maximum with changed code: Chrome 28: 2620, +100%, IE 10: 4900, +122%; FF 23: 267, +160%
var depth = 200;
var first = ko.observable(0);
var last = first;
for (var i = 0; i < depth; i++) {
(function() {
var l = last;
last = ko.computed(function() { return l() + 1; });
})();
}
var all = ko.computed(function() { return last() + first(); });
first(1);
expect(all()).toEqual(depth+2);
});
})
2 changes: 1 addition & 1 deletion src/subscribables/dependencyDetection.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ko.dependencyDetection = (function () {

return {
begin: function (callback) {
_frames.push({ callback: callback, distinctDependencies:[] });
_frames.push(callback && { callback: callback, distinctDependencies:[] });
},

end: function () {
Expand Down
2 changes: 1 addition & 1 deletion src/subscribables/dependentObservable.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
addSubscriptionToDependency(subscribable); // Brand new subscription - add it
});

var newValue = readFunction.call(evaluatorFunctionTarget);
var newValue = evaluatorFunctionTarget ? readFunction.call(evaluatorFunctionTarget) : readFunction();

// For each subscription no longer being used, remove it from the active subscriptions list and dispose it
for (var i = disposalCandidates.length - 1; i >= 0; i--) {
Expand Down
11 changes: 7 additions & 4 deletions src/subscribables/subscribable.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@ ko.subscribable['fn'] = {
"notifySubscribers": function (valueToNotify, event) {
event = event || defaultEvent;
if (this.hasSubscriptionsForEvent(event)) {
ko.dependencyDetection.ignore(function() {
ko.utils.arrayForEach(this._subscriptions[event].slice(0), function (subscription) {
try {
ko.dependencyDetection.begin();
for (var a = this._subscriptions[event].slice(0), i = 0, subscription; subscription = a[i]; ++i) {
// In case a subscription was disposed during the arrayForEach cycle, check
// for isDisposed on each subscription before invoking its callback
if (subscription && (subscription.isDisposed !== true))
subscription.callback(valueToNotify);
});
}, this);
}
} finally {
ko.dependencyDetection.end();
}
}
},

Expand Down

0 comments on commit 688a567

Please sign in to comment.