pnpm / supi

Fast, disk space efficient installation engine. Used by pnpm
MIT License
24 stars 5 forks source link

Rewrite pnpm using reactive programming #8

Closed zkochan closed 7 years ago

zkochan commented 7 years ago

Breaking changes:

zkochan commented 7 years ago

All tests pass but pnpm is 2 times slower with these changes. I can investigate it next week but maybe someone experienced with rxjs can notice errors in my "reactive code" as I am really inexperienced with rxjs

vjpr commented 7 years ago

pnpm is 2 times slower with these changes

I think this is a problem with moving to a reactive approach in general. You have much less ability to tweak the performance, without diving into the underlying library.

The benefits of reactive are not speed/performance, only (maybe) code that is easier to reason about.


Last time I tried to debug performance in pnpm with ramda it was already very difficult.

zkochan commented 7 years ago

Of course, I just want to make some really complex optimizations and it was really hard for me to abstract the code. Streams helped me with it. I won't merge/publish these changes if they will be slower than the current pnpm.

zkochan commented 7 years ago

As of now, with these changes, pnpm has the same speed as without them.

vjpr commented 7 years ago

Cool. Is it ready to test? Can you publish a beta and I will test on my massive monolith. On Fri 25. Aug 2017 at 11:11 PM, Zoltan Kochan notifications@github.com wrote:

As of now, with these changes, pnpm has the same speed as without them.

— You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub https://github.com/pnpm/supi/pull/8#issuecomment-325035376, or mute the thread https://github.com/notifications/unsubscribe-auth/AARLRaNO5Ra6N468o6wytFadSolKQxWYks5sbziVgaJpZM4OwxzN .

zkochan commented 7 years ago

I want to try to make it a little bit faster than the current version (to justify the huge changes). I will probably do this: https://github.com/pnpm/pnpm/issues/841. It will allow saving the shrinkwrap and store index files earlier.

Also, André Staltz suggested using most instead of rxjs. He says sometimes it is a lot faster than rxjs. I might try that... or maybe later as it might be a lot of work.

I'll publish a beta this weekend though.

zkochan commented 7 years ago

@vjpr I published the changes as next via pnpm@2.0.0-beta.0

zkochan commented 7 years ago

I reverted the breaking changes. They were not necessary, so this will be a minor version bump in pnpm