hsl icon indicating copy to clipboard operation
hsl copied to clipboard

Allow `Async\Semaphore` to wait for different types

Open Atry opened this issue 4 years ago • 1 comments

I use Async\Semaphore to limit the concurrency in https://github.com/hhvm/hhast/pull/399, which works good. But the current design of Async\Semaphore does not support the use cases where the waitForAsync callers expect different types. I wonder if we could support the following type signature, so that we can use the same Semaphore limit the concurrency of heterogeneous job types.

final class Semaphore {
  public function __construct(
    private int $concurrentLimit,
  );

  public async function waitForAsync<T>((function():Awaitable<T>) $value): Awaitable<T>;
}

Atry avatar Nov 23 '21 18:11 Atry

Another idea is to use IAsyncDisposable to release the worker instead of a closure.

final class Semaphore {
  public function __construct(
    private int $concurrentLimit,
  );

  public function aquireWorkerAsync(): Awaitable<IAsyncDisposable>;
}
// Usage
$worker_pool = new Semaphore(2);

async function my_script(vec<string> $files_to_lint): Awaitable<void> {
  $lint_results = Vec\map(
    $files_to_lint,
    $file ==> {
      await using($_worker = await $worker_pool->aquireWorkerAsync()) {
        return call_hh_client_linter($file);
      }
    }
  );
}

This approach is better than the closure approach because it could keep the call stack clean. In the closure approach, when an exception is thrown from the closure, the call stack would be noisy and hard to debug.

Atry avatar Nov 25 '21 19:11 Atry