<div dir="ltr">The pluggable method would at least allow everyone to continue working until someone has time to dig into what's wrong with multiprocess on Windows</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 4, 2015 at 9:56 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 4, 2015 at 5:40 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span><div dir="ltr">On Fri, Sep 4, 2015 at 5:10 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">tfiala added a comment.<br>
<br>
In <a href="http://reviews.llvm.org/D12651#240480" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12651#240480</a>, @zturner wrote:<br>
<br>
> Tried out this patch, unfortunately I'm seeing the same thing.  The very<br>
>  first call to worker.join() is never returning.<br>
><br>
> It's unfortunate that it's so hard to debug this stuff, do you have any<br>
>  suggestions for how I can try to nail down what the child dotest instance<br>
>  is actually doing?  I wonder if it's blocking somewhere in its script, or<br>
>  if this is some quirk of the multiprocessing library's dynamic invocation /<br>
>  whatever magic is does.<br>
><br>
> How much of an effort would it be to make the switch to threads now?  The<br>
>  main thing we'd have to do is get rid of all of the globals in dotest, and<br>
>  make a DoTest class or something.<br>
<br>
<br>
It's a bit more work than I want to take on right now.  I think we really may want to keep the multiprocessing and just not exec out to dotest.py for a third-ish time for each inferior.<br></blockquote><div><br></div></span><div>Just to clarify, are you saying we may want to keep multiprocessing over threads even if you can solve the exec problem?  Any particular reason?</div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Yes, you understood me correctly.</div><div><br></div><div>Prior to me getting into it, dosep.py was designed to isolate each test into its own process (via the subprocess exec call) so that each test directory or file got its own lldb processor and there was process-level isolation, less contention on the Python global interpreter lock, etc.</div><div><br></div><div>Then, when Steve Pucci and later I got to making it multithreaded, we wrapped the exec call in a "import threading" style thread pool.  That maintained the process isolation property by having each thread just do an exec (i.e. multiple execs in parallel).  Except, this didn't work on MacOSX.  The exec calls grab the Python GIL on OS X (and not anywhere as as far as I could find).  But multithreading + exec is a valid option for everything not OS X.</div><div><br></div><div>The way I solved it to work for everyone was to drop the "import threading" approach and switch to the "import multiprocessing" approach.  This worked everywhere, including on OS X (although with a few hiccups initially, as it exposed occasional hangs at the time with what looked like socket handling under Darwin).  What I failed to see in my haste was that I then had two levels of fork/exec-like behavior (i.e. we had two process firewalls where we only needed one, at the cost of an extra exec): the multiprocessing works by effectively forking/creating a new process that is now isolated.  But then we turn around and still create a subprocess to exec out to dotest.py.</div><div><br></div><div>What I'm suggesting in the near future is if we stick with the multiprocessing approach, and eliminate the subprocess exec and instead just have the multiprocess worker call directly into a methodized entry point in dotest.py, we can skip the subprocess call within the multiprocess worker.  It is already isolated and a separate process, so it is already fulfilling the isolation requirement.  And it reduces the doubled processes created.  And it works on OS X in addition to everywhere else.  It does become more difficult to debug, but then again the majority of the logic is in dotest.py and can be debugged --no-multiprocess (or with logging).</div><div><br></div><div>This is all separate somewhat from the Ctrl-C issue, but it is the backstory on what I'm referring to with the parallel test runner.</div><div><br></div><div>Completely as an aside, I did ask Greg Clayton to see if he can poke into why OS X is hitting the Python GIL on execs in "import threading"-style execs from multiple threads.  But assuming nothing magic changes there and it wasn't easily solved (I tried and failed after several attempts to diagnose last year), I'd prefer to keep a strategy that is the same unless there's a decent win on the execution front.</div><div><br></div><div>That all said, I'm starting to think a pluggable strategy for the actual mechanic of the parallel test run might end up being best anyway since I'd really like the Ctrl-C working and I'm not able to diagnose what's happening on the Windows scenario.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>  Multi-threaded is much easier to debug, for starters, because you can just attach your debugger to a single process.  It also solves a lot of race conditions and makes output processing easier (not to mention higher performance), because you don't even need a way to have the sub-processes communicate their results back to the parent because the results are just in memory.  stick them in a synchronized queue and the parent can just process it.  So it would probably even speed up the test runner.</div><div><br></div><div>I think if there's not a very good reason to keep multiprocessing around, we should aim for a threaded approach.  My understanding is that lit already does this, so there's no fundamental reason it shouldn't work correctly on MacOSX, just have to solve the exec problem like you mentioned.</div><div><br></div><div><br></div></div></div>
</blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">-Todd</div></div>
</div></div></blockquote></div>