search

purescript / psc-package

A package manager for PureScript based on package sets
https://psc-package.readthedocs.io
Other
228 stars 45 forks source link

Limit threads #127

Closed eviefp closed 6 years ago

eviefp commented 6 years ago

This is more of a "is this what you had in mind", I'm totally open to feedback.

I added a --threads N option for all commands that (eventually) hit a mapConcurrently or a forConcurrently_. I did it by adding a Maybe Int parameter down the call stack (not sure if it's worth it refactoring all of the functions to some ReaderT env IO; probably not with this PR but I could take that on if it would get accepted).

A value of Nothing means use old behaviour, otherwise use the method suggested by @kRITZCREEK's link in #126.

I would also like to keep the changes to the import because I like them arranged nicely, but I'm totally fine reverting that chunk if you want me to.

This has been tested on Appveyor on our project and fixed our issues. We have only tested with --threads 1 so far and that works (but is obviously a bit slow).

justinwoo commented 6 years ago

This sounds fine overall? Just I guess we should do a pre-release in any case if we merge this to see if anyone has problems with this.

eviefp commented 6 years ago

No rush for me, we've included the binary from this branch in our repo until it's up on chocolatey.

eviefp commented 6 years ago

Thanks for the reviews, I think I addressed the issues. Let me know if the current message is okay @justinwoo

I haven't tested it yet, will do that tomorrow at work on our codebase and will update then.

eviefp commented 6 years ago

Alright, after a bit of confusion I think everything is working correctly. Tested it on appveyor and it works just fine with up to 6 jobs. Didn't encounter any issues with this version.

eviefp commented 6 years ago

Added bracket_, made sure there's no more formatting noise and tested it on our appveyor. Seems to work even with jobs = 10, but still fails with unlimited.

If you mind the Makefile I could remove that too.

justinwoo commented 6 years ago

Thanks! I made a pre-release here: https://github.com/purescript/psc-package/releases/tag/v0.4.1-pre