Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ko.utils.arrayPushAll is inefficient when called with an observable array #256

Closed
mbest opened this issue Oct 9, 2017 · 2 comments
Closed

Comments

@mbest
Copy link
Contributor

mbest commented Oct 9, 2017

ko.utils.arrayPushAll(self.data, buildData(1000));

arrayPushAll will make 1000 push calls, which each update the binding separately. Although one could argue that arrayPushAll should work better, this code should use the recommended approach:

self.add = function () {
    startMeasure("add");
    self.data.push.apply(self.data, buildData(1000));
    stopMeasure();
};
@chrisknoll
Copy link

@mbest, not sure if this change had an impact on the results. The 'append 1k rows to large table' test still shows a very strange difference in execution time. Also, not clear if round 7 included this update, but the PR was merged in october, and Round 7 was published in november, so I'm assuming the change is in round 7.

@mbest
Copy link
Contributor Author

mbest commented Feb 22, 2018

I was wrong on how I read the Knockout code surrounding arrayPushAll, which is actually pretty much the same as push.apply. Thus the problem is actually deeper in Knockout regarding changes to large arrays. This should become better with Knockout 3.5 (knockout/knockout#2324).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants