Implement multi-core builds in MSVS

visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/compiler_wrapper.py

Index: visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/compiler_wrapper.py
diff --git a/visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/compiler_wrapper.py b/visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/compiler_wrapper.py
new file mode 100644
index 0000000000000000000000000000000000000000..a3de4e725bed701f92e129b7665d491b566ec733
--- /dev/null
+++ b/visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/compiler_wrapper.py
@@ -0,0 +1,144 @@
+#!/usr/bin/env python

[binji, 2012/10/03 22:52:13]

add license header

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+import os
+import Queue
+import shlex
+import subprocess
+import sys
+import threading
+import multiprocessing
+import time

[binji, 2012/10/03 22:52:13]

sort imports alphabetically

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+
+"""Python wrapper around gcc to make it behave a little
+more like cl.exe WRT to parallel building.
+"""
+
+verbose = int(os.environ.get('NACL_GCC_VERBOSE', '0'))
+show_commands = int(os.environ.get('NACL_GCC_SHOW_COMMANDS', '0'))
+cores = int(os.environ.get('NACL_GCC_CORES', '0'))
+stop_on_error = False
+
+def RunGCC(cmd, basename):
+  cmdstring = subprocess.list2cmdline(cmd)
+  if show_commands:

[binji, 2012/10/03 22:52:13]

nit: move down to use of logmsg

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+    logmsg = cmdstring
+  else:
+    logmsg = basename
+  p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+  stdout, stderr = p.communicate()
+  p.wait()
+  stdout = logmsg + '\n' + stdout
+  return (p.returncode, stdout, stderr)
+
+
+def BuildSerial(base_cmd, outpath, files):

[binji, 2012/10/03 22:52:13]

Is this necessary? Why not always use BuildParallel below?

[Sam Clegg, 2012/10/04 22:57:57]

Just here for debugging.  This code path is not used.
Should I remove it?

[binji, 2012/10/04 23:27:31]

Looks like it will still run if cores==1... but it seems like there is enough
duplicated logic that it would make sense to remove. Your call.

+  final_result = 0
+
+  for filename in files:
+    cmd, basename = MakeCommand(base_cmd, outpath, filename)
+    rtn, stdout, stderr = RunGCC(cmd, basename)
+    sys.stderr.write(stdout)
+    sys.stderr.write(stderr)
+    if rtn:
+      final_result = rtn
+      if stop_on_error:
+        break
+
+  return final_result
+
+
+q = Queue.Queue()

[binji, 2012/10/03 22:52:13]

IMO, it is nicer to make these local to BuildParallel and pass them via args in
threading.Thread below.

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+outq = Queue.Queue()
+
+
+def Worker():
+  while not q.empty():

[binji, 2012/10/03 22:52:13]

and Worker.running?

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+    item = q.get(False)
+    if not item:
+      break
+    results = RunGCC(item[0], item[1])
+    outq.put(results)
+
+
+def MakeCommand(base_cmd, outpath, filename):
+  basename = os.path.basename(filename)
+  out = os.path.join(outpath, os.path.splitext(basename)[0] + '.obj')

[binji, 2012/10/03 22:52:13]

use .o instead of .obj?

[Sam Clegg, 2012/10/04 22:57:57]

Unfortunately we can't in this case as we need the OutFile
to be the same for each object.

The way visual studio does this is to allow you to specify
a directory as the '-o' file in which case the object are
supposed to be created as "OutFile/<basename>.obj".

[binji, 2012/10/04 23:27:31]

ok, comment would be nice. :)

+  return (base_cmd + ['-c', filename, '-o', out], basename)
+
+
+def BuildParallel(base_cmd, outpath, files):
+  pool = []
+
+  for filename in files:
+    cmd, basename = MakeCommand(base_cmd, outpath, filename)
+    q.put((cmd, basename))
+
+  Worker.running = True
+  for i in xrange(cores):
+    t = threading.Thread(target=Worker)
+    t.start()
+
+  results = 0
+  Trace("waiting for %d results" % len(files))
+  final_result = 0
+  while results < len(files):
+    results += 1
+    rtn, stdout, stderr = outq.get()
+    sys.stderr.write(stdout)

[binji, 2012/10/03 22:52:13]

why write stdout from compiler to stderr?

[Sam Clegg, 2012/10/04 22:57:57]

Good question.  Visual studio doesn't display stdout
from cl.exe it seems.  Only shows stderr.  Do we case if
GCC stdout is lost?  I imagine all the important stuff is
on stderr already right?   As it stands I link we are already loosing stdout.

[binji, 2012/10/04 23:27:31]

OK, might add a comment then.

[Sam Clegg, 2012/10/05 21:11:19]

Done.

+    sys.stderr.write(stderr)
+    if rtn:
+      final_result = rtn
+      if stop_on_error:
+        # stop all workers
+        Worker.running = False
+        break
+
+  return final_result
+
+
+def Log(msg):
+  sys.stderr.write(str(msg) + '\n')
+  sys.stderr.flush()
+
+
+def Trace(msg):
+  if verbose:
+    Log(msg)
+
+
+def main(args):
+  global cores
+  if args[0][0] == '@':
+    rspfile = args[0][1:]
+    args = shlex.split(open(rspfile).read())
+
+  # find the last occurance of '--' in the argument

[binji, 2012/10/03 22:52:13]

nit: s/occurance/occurrence/

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+  # list an use that to signify the start of the

[binji, 2012/10/03 22:52:13]

s/an/and/

[Sam Clegg, 2012/10/04 22:57:57]

Done.

+  # list of sources
+  index = list(reversed(args)).index('--')
+  index = len(args) - index
+  base_cmd = args[:index-1]
+  files = args[index:]
+
+  # remove -o <path> from base_cmd
+  index = base_cmd.index('-o')
+  outpath = base_cmd[index+1]
+  del base_cmd[index+1]
+  del base_cmd[index]
+
+  if not cores:
+    cores = multiprocessing.cpu_count()
+
+  cores = min(cores, len(files))
+
+  Trace("compiling %d sources using %d threads" % (len(files), cores))
+
+  if cores == 1:
+    rtn = BuildSerial(base_cmd, outpath, files)
+  else:
+    rtn = BuildParallel(base_cmd, outpath, files)
+
+  Trace("done build of %d sources" % (len(files)))

[binji, 2012/10/03 22:52:13]

what about when the build fails?

[Sam Clegg, 2012/10/04 22:57:57]

Well spotted.  Amazingly (you are not going to believe this)
visual studio seems to look at the error messages on stdout
to decide if the build failed and not at the return code.

I will fix this anyway... and double confirm what I am seeing.

[binji, 2012/10/04 23:27:31]

Haha, I actually meant the Trace is displaying that the build finished the
sources, even though it may not have.

[Sam Clegg, 2012/10/05 21:11:19]

Done.

+
+
+if __name__ == '__main__':
+  main(sys.argv[1:])