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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to