[PATCH] D69582: Let clang driver support parallel jobs

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

This is somehow similar to what I was proposing in D52193 <https://reviews.llvm.org/D52193>.

Would you possibly provide tests and/or an example of your usage please?



================
Comment at: clang/lib/Driver/Compilation.cpp:303
+    }
+    std::thread Th(Work);
+    Th.detach();
----------------
thakis wrote:
> Maybe a select() / fork() loop is a better approach than spawning one thread per subprocess? This is doing thread-level parallelism in addition to process-level parallelism :)
> 
> If llvm doesn't have a subprocess pool abstraction yet, ninja's is pretty small, self-contained, battle-tested and open-source, maybe you could copy that over (and remove bits you don't need)?
> 
> https://github.com/ninja-build/ninja/blob/master/src/subprocess.h
> https://github.com/ninja-build/ninja/blob/master/src/subprocess-win32.cc
> https://github.com/ninja-build/ninja/blob/master/src/subprocess-posix.cc
@thakis How would this new `Subprocess` interface with the existing `llvm/include/llvm/Support/Program.h` APIs? Wouldn't be better to simply extend what is already there with a `WaitMany()` and a `Terminate()` API like I was suggesting in D52193? That would cover all that's needed. Or are you suggesting to further stub `ExecuteAndWait()` by this new `Subprocess` API?


================
Comment at: clang/lib/Driver/Compilation.cpp:332
+    if (!Next) {
+      std::this_thread::yield();
       continue;
----------------
In addition to what @thakis said above, yielding here is maybe not a good idea. This causes the process to spin, and remain in the OS' active process list, which uselessly eats cpu cycles. This can become significant over the course of several minutes of compilation.

Here's a //tiny// example of what happens when threads are waiting for something to happen:
(the top parts yields frequently; the bottom part does not yield - see D68820)
{F10592208}

You would need here to go through a OS primitive that suspends the process until at least one job in the pool completes. On Windows this can be achieved through `WaitForMultipleObjects()` or I/O completion ports like provided by @thakis. You can take a look at `Compilation::executeJobs()` in D52193 and further down the line, `WaitMany()` which waits for at least one job/process to complete.


================
Comment at: clang/lib/Driver/Compilation.cpp:354
+    };
+    JS.launch(Work);
   }
----------------
It's a waste to start a new thread here just because `ExecuteAndWait()` is used inside `Command::Execute()`. An async mechanism would be a lot better like stated above.


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

https://reviews.llvm.org/D69582