[PATCH] D102633: [clang-scan-deps] Improvements to thread usage

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a subscriber: jansvoboda11.
dexonsmith added a comment.

In D102633#2773578 <https://reviews.llvm.org/D102633#2773578>, @aganea wrote:

> In D102633#2769762 <https://reviews.llvm.org/D102633#2769762>, @arphaman wrote:
>
>> It might be good for @aganea to take a look as well.
>
> Thanks! I actually work with @saudi, I already took a look at the patch before uploading. However I'm stil torn about running one of the workers on the main thread. I fear that we could have random errors because of the stack size of the "main" thread could be different from the stack size of the "satellite" threads. There's 99.99% chance that this won't happen, but I'd prefer that behavior to be explicit.
>
> We could have:
>
>   -j0: use all hardware threads
>   -j1: don't use multi-threading, run all the tasks on the main thread
>   -jN: use the specified number of threads
>
> The rationale is that we're using clang-scan-deps as-a-DLL in Fastbuild, to extract the dependencies. Since Fastbuild already has its own thread pool management, we call into clang-scan-deps with -j1 from different Fastbuild threads (but keeping the `DependencyScanningService` alive between calls). It would great if each call to clang-scan-deps wouldn't create a extra new thread.
>
> Perhaps any of you would like to comment? + at mehdi_amini

Seems like if you want/need a DLL you should be using a library interface not the tool interface. If you can't / don't want to use the C++ API in clang/lib/Tooling, then we should push out a C API. Maybe you can confirm that https://github.com/apple/llvm-project/blob/apple/main/clang/include/clang-c/Dependencies.h in libclang will work for your needs? It landed on the fork just because it was experimental and libclang has a stable API promise, but I think the names of the functions make that clear enough -- e.g., `clang_experimental_DependencyScannerWorker_create_v0` seems well-enough named that no one expects permanent compatibility -- and it seems pretty awkward to be using the tool as a DLL to work around not having this in tree. Then your FastBuild can directly manage the service and the workers. WDYT?

@arphaman / @Bigcheese / @jansvoboda11 , thoughts?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102633/new/

https://reviews.llvm.org/D102633