Merge pull request SCons#4276 from bdbaddog/add_new_parallel_job_expe…

…rimental_feature

Added --experimental=tm_v2 which switches to use Andrew Morrow's new ParallelJob implementation.

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).

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
 ------------------------

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:

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,17 +84,24 @@ 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
             if stack_size is None:
                 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

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

doc/man/scons.xml

@@ -1125,11 +1125,12 @@ the mechanisms in the specified order.</para>
         the special tokens <literal>all</literal> or <literal>none</literal>.
         A comma-separated string can be used to select multiple features.
         The default setting is <literal>none</literal>.</para>
-      <para>Current available features are: <literal>ninja</literal>.</para>
+      <para>Current available features are: <literal>ninja</literal>, <literal>tm_v2</literal>.</para>
       <caution><para>
         No Support offered for any features or tools enabled by this flag.
       </para></caution>
-      <para><emphasis>Available since &scons; 4.2.</emphasis></para>
+        <para><emphasis><literal>ninja</literal> Available since &scons; 4.2.</emphasis></para>
+        <para><emphasis><literal>tm_v2</literal> Available since &scons; 4.4.1</emphasis></para>
     </listitem>
   </varlistentry>
 

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)
 

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+\]')