diff --git a/CHANGES.txt b/CHANGES.txt index 5e7f6d4a4e..f2a42e289d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -32,6 +32,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Moved rpm and debian directories under packaging - Added logic to help packagers enable reproducible builds into packaging/etc/. Please read packaging/etc/README.txt if you are interested. + - Added --experimental=tm_v2, which enables Andrew Morrow's new NewParallel Job implementation. + This should scale much better for highly parallel builds. You can also enable this via SetOption(). From Dan Mezhiborsky: - Add newline to end of compilation db (compile_commands.json). diff --git a/RELEASE.txt b/RELEASE.txt index 801062a9ed..2a32831407 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -24,6 +24,9 @@ NEW FUNCTIONALITY - Added ValidateOptions() which will check that all command line options are in either those specified by SCons itself, or by AddOption() in SConstruct/SConscript. It should not be called until all AddOption() calls are completed. Resolves Issue #4187 +- Added --experimental=tm_v2, which enables Andrew Morrow's NewParallel Job implementation. + This should scale much better for highly parallel builds. You can also enable this via SetOption(). + DEPRECATED FUNCTIONALITY ------------------------ diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py index f3b4708c52..a3e3ea8b36 100644 --- a/SCons/Script/SConsOptions.py +++ b/SCons/Script/SConsOptions.py @@ -40,7 +40,7 @@ diskcheck_all = SCons.Node.FS.diskcheck_types() -experimental_features = {'warp_speed', 'transporter', 'ninja'} +experimental_features = {'warp_speed', 'transporter', 'ninja', 'tm_v2'} def diskcheck_convert(value): @@ -65,7 +65,7 @@ def diskcheck_convert(value): class SConsValues(optparse.Values): """ Holder class for uniform access to SCons options, regardless - of whether or not they can be set on the command line or in the + of whether they can be set on the command line or in the SConscript files (using the SetOption() function). A SCons option value can originate three different ways: diff --git a/SCons/Taskmaster/Job.py b/SCons/Taskmaster/Job.py index 853fe6387f..a63b5291bb 100644 --- a/SCons/Taskmaster/Job.py +++ b/SCons/Taskmaster/Job.py @@ -40,6 +40,7 @@ import SCons.Errors import SCons.Warnings + # The default stack size (in kilobytes) of the threads used to execute # jobs in parallel. # @@ -53,8 +54,6 @@ interrupt_msg = 'Build interrupted.' -USE_NEW_PARALLEL = os.environ.get('SCONS_NEW_PARALLEL', False) - class InterruptState: def __init__(self): self.interrupted = False @@ -85,6 +84,11 @@ class can't do it, it gets reset to 1. Wrapping interfaces that care should check the value of 'num_jobs' after initialization. """ + # Importing GetOption here instead of at top of file to avoid + # circular imports + # pylint: disable=import-outside-toplevel + from SCons.Script import GetOption + self.job = None if num > 1: stack_size = explicit_stack_size @@ -92,10 +96,12 @@ class can't do it, it gets reset to 1. Wrapping interfaces that stack_size = default_stack_size try: - if USE_NEW_PARALLEL: + experimental_option = GetOption('experimental') + if 'tm_v2' in experimental_option: self.job = NewParallel(taskmaster, num, stack_size) else: self.job = LegacyParallel(taskmaster, num, stack_size) + self.num_jobs = num except NameError: pass diff --git a/SCons/Taskmaster/JobTests.py b/SCons/Taskmaster/JobTests.py index 57b548c4d2..9e7b080246 100644 --- a/SCons/Taskmaster/JobTests.py +++ b/SCons/Taskmaster/JobTests.py @@ -24,12 +24,10 @@ import unittest import random import math -import sys import os -import TestUnit - import SCons.Taskmaster.Job +from SCons.Script.Main import OptionsParser def get_cpu_nums(): @@ -244,10 +242,25 @@ def exception_set(self): def cleanup(self): pass + SaveThreadPool = None ThreadPoolCallList = [] -class ParallelTestCase(unittest.TestCase): + +class JobTestCase(unittest.TestCase): + """ + Setup common items needed for many Job test cases + """ + def setUp(self) -> None: + """ + Simulating real options parser experimental value. + Since we're in a unit test we're actually using FakeOptionParser() + Which has no values and no defaults. + """ + OptionsParser.values.experimental = [] + + +class ParallelTestCase(JobTestCase): def runTest(self): """test parallel jobs""" @@ -334,7 +347,9 @@ def runTest(self): self.assertFalse(taskmaster.num_failed, "some task(s) failed to execute") -class NoParallelTestCase(unittest.TestCase): + +class NoParallelTestCase(JobTestCase): + def runTest(self): """test handling lack of parallel support""" def NoParallel(tm, num, stack_size): @@ -378,7 +393,9 @@ def runTest(self): self.assertTrue(taskmaster.num_postprocessed == 1, "exactly one task should have been postprocessed") -class ParallelExceptionTestCase(unittest.TestCase): + +class ParallelExceptionTestCase(JobTestCase): + def runTest(self): """test parallel jobs with tasks that raise exceptions""" @@ -447,7 +464,8 @@ class badpreparenode (badnode): def prepare(self): raise Exception('badpreparenode exception') -class _SConsTaskTest(unittest.TestCase): + +class _SConsTaskTest(JobTestCase): def _test_seq(self, num_jobs): for node_seq in [ @@ -541,31 +559,19 @@ def runTest(self): """test parallel jobs with actual Taskmaster and Task""" self._test_seq(num_jobs) + # Now run test with NewParallel() instead of LegacyParallel + OptionsParser.values.experimental=['tm_v2'] + self._test_seq(num_jobs) -#--------------------------------------------------------------------- -def suite(): - suite = unittest.TestSuite() - suite.addTest(ParallelTestCase()) - suite.addTest(SerialTestCase()) - suite.addTest(NoParallelTestCase()) - suite.addTest(SerialExceptionTestCase()) - suite.addTest(ParallelExceptionTestCase()) - suite.addTest(SerialTaskTest()) - suite.addTest(ParallelTaskTest()) - return suite + +#--------------------------------------------------------------------- if __name__ == "__main__": - runner = TestUnit.cli.get_runner() - result = runner().run(suite()) - if (len(result.failures) == 0 - and len(result.errors) == 1 - and isinstance(result.errors[0][0], SerialTestCase) - and isinstance(result.errors[0][1][0], NoThreadsException)): - sys.exit(2) - elif not result.wasSuccessful(): - sys.exit(1) + unittest.main() + + # Local Variables: # tab-width:4 diff --git a/doc/man/scons.xml b/doc/man/scons.xml index 0278d8793d..2bdf2b92f1 100644 --- a/doc/man/scons.xml +++ b/doc/man/scons.xml @@ -1125,11 +1125,12 @@ the mechanisms in the specified order. the special tokens all or none. A comma-separated string can be used to select multiple features. The default setting is none. - Current available features are: ninja. + Current available features are: ninja, tm_v2. No Support offered for any features or tools enabled by this flag. - Available since &scons; 4.2. + ninja Available since &scons; 4.2. + tm_v2 Available since &scons; 4.4.1 diff --git a/test/option/fixture/SConstruct__taskmastertrace.py b/test/option/fixture/SConstruct__taskmastertrace similarity index 100% rename from test/option/fixture/SConstruct__taskmastertrace.py rename to test/option/fixture/SConstruct__taskmastertrace diff --git a/test/option/option--experimental.py b/test/option/option--experimental.py index 51f3546c3d..324de99d24 100644 --- a/test/option/option--experimental.py +++ b/test/option/option--experimental.py @@ -36,12 +36,13 @@ tests = [ ('.', []), ('--experimental=ninja', ['ninja']), - ('--experimental=all', ['ninja', 'transporter', 'warp_speed']), + ('--experimental=tm_v2', ['tm_v2']), + ('--experimental=all', ['ninja', 'tm_v2', 'transporter', 'warp_speed']), ('--experimental=none', []), ] for args, exper in tests: - read_string = """All Features=ninja,transporter,warp_speed + read_string = """All Features=ninja,tm_v2,transporter,warp_speed Experimental=%s """ % (exper) test.run(arguments=args, @@ -50,7 +51,7 @@ test.run(arguments='--experimental=warp_drive', stderr="""usage: scons [OPTIONS] [VARIABLES] [TARGETS] -SCons Error: option --experimental: invalid choice: 'warp_drive' (choose from 'all','none','ninja','transporter','warp_speed') +SCons Error: option --experimental: invalid choice: 'warp_drive' (choose from 'all','none','ninja','tm_v2','transporter','warp_speed') """, status=2) diff --git a/test/option/taskmastertrace.py b/test/option/taskmastertrace.py index 0e44896231..99de718514 100644 --- a/test/option/taskmastertrace.py +++ b/test/option/taskmastertrace.py @@ -33,7 +33,7 @@ test = TestSCons.TestSCons() -test.file_fixture('fixture/SConstruct__taskmastertrace.py', 'SConstruct') +test.file_fixture('fixture/SConstruct__taskmastertrace', 'SConstruct') test.file_fixture('fixture/taskmaster_expected_stdout_1.txt', 'taskmaster_expected_stdout_1.txt') test.file_fixture('fixture/taskmaster_expected_file_1.txt', 'taskmaster_expected_file_1.txt') test.file_fixture('fixture/taskmaster_expected_new_parallel.txt', 'taskmaster_expected_new_parallel.txt') @@ -55,8 +55,7 @@ test.must_match_file('trace.out', 'taskmaster_expected_file_1.txt', mode='r') # Test NewParallel Job implementation -os.environ['SCONS_NEW_PARALLEL'] = "1" -test.run(arguments='-j 2 --taskmastertrace=new_parallel_trace.out .') +test.run(arguments='-j 2 --experimental=tm_v2 --taskmastertrace=new_parallel_trace.out .') new_trace = test.read('new_parallel_trace.out', mode='r') thread_id = re.compile(r'\[Thread:\d+\]')