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

Fix performance for diamond dependencies #41

Open
gerich-home opened this issue Mar 24, 2016 · 6 comments
Open

Fix performance for diamond dependencies #41

gerich-home opened this issue Mar 24, 2016 · 6 comments
Assignees

Comments

@gerich-home
Copy link
Owner

See test from ee7a353

Perf results on my local machine are:

 'computed diamond updated 1000 times with 500 hidden dependencies'
     'knockout' is etalon
     'itDepends' at 1066.24x slower
@gerich-home
Copy link
Owner Author

Seems that the solution is to move postpone the resubscribing (https://github.com/gerich-home/it-depends/blob/master/src/computed.ts#L113) until value.write ends.
However this requires redisigning notification code a bit

@gerich-home gerich-home self-assigned this Mar 25, 2016
@VanDalkvist
Copy link
Collaborator

'computed diamond updated 1000 times with 500 hidden dependencies' 
Passed:
     'knockout' is etalon
     'itDepends' at 2803.88x slower

2803.88x - 😞

@gerich-home
Copy link
Owner Author

Yeah.. Working on it, has some ideas, similar in design to bulkChange.ts

@VanDalkvist
Copy link
Collaborator

Maybe I'm wrong but it seems you forgot

      e.subscribe(function () {
      });

for knockout setup after

var e = ko.pureComputed(function() {
       return d() % 2 == 0 ? b() : c();
});

Or it was in purpose? Because you have

var s = e.onChange(function(){
});

for it-depends setup

@VanDalkvist
Copy link
Collaborator

In this case performance logs become more real:

'computed diamond updated 1000 times with 500 hidden dependencies' (passed: 2, failed: 0)
     Passed:
       'knockout' is etalon
       'itDepends' at 56.81x slower
Finished 'performance-test-computedDiamondUpdates' after 36 s

@gerich-home
Copy link
Owner Author

Correct... found it independently and pushed to branch diamond-performance

This was referenced Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants