[llvm-commits] [PATCH] [Lit] Use multiprocessing instead of threading

[Eli Bendersky]

On Fri, Nov 23, 2012 at 12:03 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
>
> Threading is too slow possibly due to GIL.
> multiprocessing brings 95% of processor usage. I believe it would scale up!
>

Sorry I missed this patch. I'm +10 for this change, as I believe
threading is much less useful than multiprocessing in this case for
true scaling. While we're at it, can we consider requiring Python 2.6?
It was released more than 4 years ago! I realize some systems may come
with an older Python out of the box, but can't we require users to
install Python 2.6 in order to run LLVM tests? Are there any major
platforms for which this is a problem?

Some minor comments on the patch below:

+def runMPTests(numThreads, litConfig, tests, display):
+

Should the parameter still be called numThreads?

+def runMPTests(numThreads, litConfig, tests, display):
+
+    import multiprocessing
+

I personally prefer to avoid function-level imports, since they make
the code harder to analyze (e.g. see at a glance which modules are
being used by some file). It also makes later logic more cumbersome:

+    try:
+        if platform.system()=='Windows':
+            # It's still slow to invoke children due to cost of pickling.
+            pass
+        else:
+            runMPTests(numThreads, litConfig, tests, display)
+            return
+    except ImportError:
+        pass

Why should "running" fail with an "import error"?

I think it would be clearer to have something like this at top level:

have_MP = True
try:
  import multiprocessing
except ImportError:
  have_MP = false

Or something similar, and then your condition would be:

+        if platform.system()=='Windows':
+            # It's still slow to invoke children due to cost of pickling.
+            pass
+        elif have_MP:
+            runMPTests(numThreads, litConfig, tests, display)
+            return

Eli