Fix parallel execution, and enable by default
Fix a few issues with parallel execution:
- exit code 1 on failure
- print a useful backtrace (it looks like a regular non-parallel gyp backtrace now)
- make output match non-parallel gyp more closely
- close() and join() multiprocessing.Pool properly on completion to avoid shutting down during garbage collection when the context is already gone. Avoids occasional stderr output "Exception RuntimeError: RuntimeError('cannot join current thread',) in <Finalize object, dead> ignored".
Instead of setting a callback for filelists, pass the paths required to build the filelist path. This lets multiprocessing pass the required data to subprocesses.
And, since tests now pass, turn on by default.
The command line flag is kept and inverted to --no-parallel which is used for running tests in "gypd" format. Those are dumps of the internal state, and when the input files are parsed in parallel the ordering in the dicts changes. This only applies to 4 tests in the test/variables/ subdirectory.
The GYP_PARALLEL environment variable is no longer referenced.
For Chromium on Windows, this reduces build\gyp_chromium time from 1m13s to 36s.
R=dmazzoni@chromium.org, thakis@chromium.org
BUG=309824,154205
Review URL: https://codereview.chromium.org/38313004
git-svn-id: http://gyp.googlecode.com/svn/trunk@1773 78cadc50-ecff-11dd-a971-7dbc132099af
diff --git a/pylib/gyp/__init__.py b/pylib/gyp/__init__.py
index f3de408..30edea5 100755
--- a/pylib/gyp/__init__.py
+++ b/pylib/gyp/__init__.py
@@ -119,8 +119,8 @@
'generator_wants_static_library_dependencies_adjusted', True),
'generator_wants_sorted_dependencies':
getattr(generator, 'generator_wants_sorted_dependencies', False),
- 'generator_filelist_path':
- getattr(generator, 'generator_filelist_path', None),
+ 'generator_filelist_paths':
+ getattr(generator, 'generator_filelist_paths', None),
}
# Process the input specific to this generator.
@@ -324,9 +324,8 @@
parser.add_option('--no-circular-check', dest='circular_check',
action='store_false', default=True, regenerate=False,
help="don't check for circular relationships between files")
- parser.add_option('--parallel', action='store_true',
- env_name='GYP_PARALLEL',
- help='Use multiprocessing for speed (experimental)')
+ parser.add_option('--no-parallel', action='store_true', default=False,
+ help='Disable multiprocessing')
parser.add_option('-S', '--suffix', dest='suffix', default='',
help='suffix to add to generated files')
parser.add_option('--toplevel-dir', dest='toplevel_dir', action='store',
@@ -389,9 +388,7 @@
if g_o:
options.generator_output = g_o
- if not options.parallel and options.use_environment:
- p = os.environ.get('GYP_PARALLEL')
- options.parallel = bool(p and p != '0')
+ options.parallel = not options.no_parallel
for mode in options.debug:
gyp.debug[mode] = 1
diff --git a/pylib/gyp/generator/ninja.py b/pylib/gyp/generator/ninja.py
index d1a6880..37208d4 100644
--- a/pylib/gyp/generator/ninja.py
+++ b/pylib/gyp/generator/ninja.py
@@ -58,7 +58,7 @@
generator_additional_non_configuration_keys = []
generator_additional_path_sections = []
generator_extra_sources_for_rules = []
-generator_filelist_path = None
+generator_filelist_paths = None
# TODO: figure out how to not build extra host objects in the non-cross-compile
# case when this is enabled, and enable unconditionally.
@@ -1483,17 +1483,11 @@
qualified_out_dir = os.path.normpath(os.path.join(
toplevel, ComputeOutputDir(params), 'gypfiles'))
- def gypfile_path(build_file_dir, name):
- # build_file_dir is absolute, make it relative to toplevel
- if os.path.isabs(build_file_dir):
- build_file_dir = gyp.common.RelativePath(build_file_dir, toplevel)
- name = os.path.join(qualified_out_dir, build_file_dir, name)
- if not os.path.isdir(os.path.dirname(name)):
- os.makedirs(os.path.dirname(name))
- return name
-
- global generator_filelist_path
- generator_filelist_path = gypfile_path
+ global generator_filelist_paths
+ generator_filelist_paths = {
+ 'toplevel': toplevel,
+ 'qualified_out_dir': qualified_out_dir,
+ }
def OpenOutput(path, mode='w'):
diff --git a/pylib/gyp/input.py b/pylib/gyp/input.py
index a08cce9..45e791d 100644
--- a/pylib/gyp/input.py
+++ b/pylib/gyp/input.py
@@ -22,6 +22,7 @@
import sys
import threading
import time
+import traceback
from gyp.common import GypError
@@ -109,8 +110,11 @@
# Controls whether or not the generator supports multiple toolsets.
multiple_toolsets = False
-# Converts filelist paths to output paths.
-generator_filelist_path = None
+# Paths for converting filelist paths to output paths: {
+# toplevel,
+# qualified_output_dir,
+# }
+generator_filelist_paths = None
def GetIncludedBuildFiles(build_file_path, aux_data, included=None):
"""Return a list of all build files included into build_file_path.
@@ -443,7 +447,8 @@
def CallLoadTargetBuildFile(global_flags,
build_file_path, data,
aux_data, variables,
- includes, depth, check):
+ includes, depth, check,
+ generator_input_info):
"""Wrapper around LoadTargetBuildFile for parallel processing.
This wrapper is used when LoadTargetBuildFile is executed in
@@ -461,6 +466,7 @@
data_keys = set(data)
aux_data_keys = set(aux_data)
+ SetGeneratorGlobals(generator_input_info)
result = LoadTargetBuildFile(build_file_path, data,
aux_data, variables,
includes, depth, check, False)
@@ -486,8 +492,12 @@
data_out,
aux_data_out,
dependencies)
+ except GypError, e:
+ sys.stderr.write("gyp: %s\n" % e)
+ return None
except Exception, e:
- print >>sys.stderr, 'Exception: ', e
+ print >>sys.stderr, 'Exception:', e
+ print >>sys.stderr, traceback.format_exc()
return None
@@ -549,7 +559,8 @@
def LoadTargetBuildFilesParallel(build_files, data, aux_data,
- variables, includes, depth, check):
+ variables, includes, depth, check,
+ generator_input_info):
parallel_state = ParallelState()
parallel_state.condition = threading.Condition()
# Make copies of the build_files argument that we can modify while working.
@@ -563,12 +574,6 @@
parallel_state.condition.acquire()
while parallel_state.dependencies or parallel_state.pending:
if parallel_state.error:
- print >>sys.stderr, (
- '\n'
- 'Note: an error occurred while running gyp using multiprocessing.\n'
- 'For more verbose output, set GYP_PARALLEL=0 in your environment.\n'
- 'If the error only occurs when GYP_PARALLEL=1, '
- 'please report a bug!')
break
if not parallel_state.dependencies:
parallel_state.condition.wait()
@@ -591,16 +596,20 @@
CallLoadTargetBuildFile,
args = (global_flags, dependency,
data_in, aux_data_in,
- variables, includes, depth, check),
+ variables, includes, depth, check, generator_input_info),
callback = parallel_state.LoadTargetBuildFileCallback)
except KeyboardInterrupt, e:
parallel_state.pool.terminate()
raise e
parallel_state.condition.release()
- if parallel_state.error:
- sys.exit()
+ parallel_state.pool.close()
+ parallel_state.pool.join()
+ parallel_state.pool = None
+
+ if parallel_state.error:
+ sys.exit(1)
# Look for the bracket that matches the first bracket seen in a
# string, and return the start and end as a tuple. For example, if
@@ -803,7 +812,19 @@
if os.path.isabs(replacement):
raise GypError('| cannot handle absolute paths, got "%s"' % replacement)
- path = generator_filelist_path(build_file_dir, replacement)
+ if not generator_filelist_paths:
+ path = os.path.join(build_file_dir, replacement)
+ else:
+ if os.path.isabs(build_file_dir):
+ toplevel = generator_filelist_paths['toplevel']
+ rel_build_file_dir = gyp.common.RelativePath(build_file_dir, toplevel)
+ else:
+ rel_build_file_dir = build_file_dir
+ qualified_out_dir = generator_filelist_paths['qualified_out_dir']
+ path = os.path.join(qualified_out_dir, rel_build_file_dir, replacement)
+ if not os.path.isdir(os.path.dirname(path)):
+ os.makedirs(os.path.dirname(path))
+
replacement = gyp.common.RelativePath(path, build_file_dir)
f = gyp.common.WriteOnDiff(path)
for i in contents_list[1:]:
@@ -2622,10 +2643,9 @@
used[key] = gyp
-def Load(build_files, variables, includes, depth, generator_input_info, check,
- circular_check, parallel, root_targets):
+def SetGeneratorGlobals(generator_input_info):
# Set up path_sections and non_configuration_keys with the default data plus
- # the generator-specifc data.
+ # the generator-specific data.
global path_sections
path_sections = base_path_sections[:]
path_sections.extend(generator_input_info['path_sections'])
@@ -2638,11 +2658,13 @@
multiple_toolsets = generator_input_info[
'generator_supports_multiple_toolsets']
- global generator_filelist_path
- generator_filelist_path = generator_input_info['generator_filelist_path']
- if not generator_filelist_path:
- generator_filelist_path = lambda a, b: os.path.join(a, b)
+ global generator_filelist_paths
+ generator_filelist_paths = generator_input_info['generator_filelist_paths']
+
+def Load(build_files, variables, includes, depth, generator_input_info, check,
+ circular_check, parallel, root_targets):
+ SetGeneratorGlobals(generator_input_info)
# A generator can have other lists (in addition to sources) be processed
# for rules.
extra_sources_for_rules = generator_input_info['extra_sources_for_rules']
@@ -2661,7 +2683,8 @@
build_files = set(map(os.path.normpath, build_files))
if parallel:
LoadTargetBuildFilesParallel(build_files, data, aux_data,
- variables, includes, depth, check)
+ variables, includes, depth, check,
+ generator_input_info)
else:
for build_file in build_files:
try:
diff --git a/test/errors/gyptest-errors.py b/test/errors/gyptest-errors.py
index eed7b24..446607f 100755
--- a/test/errors/gyptest-errors.py
+++ b/test/errors/gyptest-errors.py
@@ -22,10 +22,9 @@
test.run_gyp('duplicate_targets.gyp', status=1, stderr=stderr,
match=TestCmd.match_re)
-stderr = ('gyp: Unable to find targets in build file .*missing_targets.gyp '
- 'while trying to load missing_targets.gyp\n')
+stderr = ('.*: Unable to find targets in build file .*missing_targets.gyp.*')
test.run_gyp('missing_targets.gyp', status=1, stderr=stderr,
- match=TestCmd.match_re)
+ match=TestCmd.match_re_dotall)
stderr = ('gyp: rule bar exists in duplicate, target '
'.*duplicate_rule.gyp:foo#target\n')
@@ -33,10 +32,9 @@
match=TestCmd.match_re)
stderr = ("gyp: Key 'targets' repeated at level 1 with key path '' while "
- "reading .*duplicate_node.gyp while trying to load "
- "duplicate_node.gyp\n")
+ "reading .*duplicate_node.gyp.*")
test.run_gyp('duplicate_node.gyp', '--check', status=1, stderr=stderr,
- match=TestCmd.match_re)
+ match=TestCmd.match_re_dotall)
stderr = 'gyp: Duplicate basenames in sources section, see list above\n'
test.run_gyp('duplicate_basenames.gyp', status=1, stderr=stderr)
diff --git a/test/lib/TestGyp.py b/test/lib/TestGyp.py
index bd9b9ea..66c5ece 100644
--- a/test/lib/TestGyp.py
+++ b/test/lib/TestGyp.py
@@ -93,6 +93,7 @@
else:
gyp = 'gyp'
self.gyp = os.path.abspath(gyp)
+ self.no_parallel = False
self.initialize_build_tool()
@@ -254,6 +255,8 @@
# TODO: --depth=. works around Chromium-specific tree climbing.
depth = kw.pop('depth', '.')
run_args = ['--depth='+depth, '--format='+self.format, gyp_file]
+ if self.no_parallel:
+ run_args += ['--no-parallel']
run_args.extend(self.extra_args)
run_args.extend(args)
return self.run(program=self.gyp, arguments=run_args, **kw)
@@ -356,6 +359,11 @@
internal data structure as pretty-printed Python).
"""
format = 'gypd'
+ def __init__(self, gyp=None, *args, **kw):
+ super(TestGypGypd, self).__init__(*args, **kw)
+ # gypd implies the use of 'golden' files, so parallelizing conflicts as it
+ # causes ordering changes.
+ self.no_parallel = True
class TestGypCustom(TestGypBase):
diff --git a/test/variables/filelist/gyptest-filelist.py b/test/variables/filelist/gyptest-filelist.py
index 288b1d0..84a6cba 100755
--- a/test/variables/filelist/gyptest-filelist.py
+++ b/test/variables/filelist/gyptest-filelist.py
@@ -25,3 +25,5 @@
print "Unexpected contents of `src/dummy_foo'"
test.diff(expect, contents, 'src/dummy_foo')
test.fail_test()
+
+test.pass_test()
