Tyrrrz / CliWrap

Library for running command-line processes
MIT License
4.32k stars 264 forks source link

Support for async throttling? #151

Closed NightProgramming closed 2 years ago

NightProgramming commented 2 years ago

Details

This is just an idea and is probably out-of-scope. But what I thought is that maybe it would be nice if this library would support async throttle mechanisms, e.g. being able to define that only two processes can run at the same time.

This could be a global limit (meaning it applies to any process started with CliWrap, or an exe specific limit (limitting based on the exe path, e.g. only two processes are allowed to run at the same time, that originate from a specific exe path), or it could be a custom limit (meaning the CliWrap caller provides a throttling key, that can apply to different processes, which might be useful if different kinds of processes access the same ressource).

If the parallel processes limit is set to 1 the throttling would essentially become async locking, which might also be useful for certain kinds of processes.

Throttling could also support timeouts, meaning throwing an exception if a throttled process wasn't started within time, because other (also throttled) processes were already running and take longer to exit than the time limit of the process that wasn't started yet but only queued.

A way to implement this could be with using SemaphoreSlim because it has WaitAsync and also cancellation support.

I know that one can do this whole throttling by building something around CliWrap themselves (e.g. a wrapper that internally calls the CliWrap methods e.g. ExecuteAsync). But of course it would be nice if this could be included and might also be better for performance, because more granular throttling could be applied (e.g. a process / ProcessStartInfo can be set up before applying the throttle so that the work that is done in a throttled context is only the actual process execution, not the things around it).

Tyrrrz commented 2 years ago

Hi. I don't think it makes sense to put it into the library. There are many alternative usage scenarios (as you described) and supporting all of them would be much harder and less flexible than just using SemaphoreSlim on the calling side (also, as you described). Creating ProcessStartInfo and all related work is not significant enough to worry about.