[Git][ghc/ghc][master] 2 commits: testsuite: remove config.use_threads

Marge Bot (@marge-bot) gitlab at gitlab.haskell.org
Wed Feb 8 23:42:35 UTC 2023



Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC


Commits:
633f2799 by Cheng Shao at 2023-02-08T18:42:16-05:00
testsuite: remove config.use_threads

This patch simplifies the testsuite driver by removing the use_threads
config field. It's just a degenerate case of threads=1.

- - - - -
ca6673e3 by Cheng Shao at 2023-02-08T18:42:16-05:00
testsuite: use concurrent.futures.ThreadPoolExecutor in the driver

The testsuite driver used to create one thread per test case, and
explicitly use semaphore and locks for rate limiting and
synchronization. This is a bad practice in any language, and
occasionally may result in livelock conditions (e.g. #22889). This
patch uses concurrent.futures.ThreadPoolExecutor for scheduling test
case runs, which is simpler and more robust.

- - - - -


4 changed files:

- testsuite/driver/runtests.py
- testsuite/driver/testglobals.py
- testsuite/driver/testlib.py
- testsuite/driver/testutil.py


Changes:

=====================================
testsuite/driver/runtests.py
=====================================
@@ -26,7 +26,9 @@ from pathlib import Path
 # So we import it here first, so that the testsuite doesn't appear to fail.
 import subprocess
 
-from testutil import getStdout, Watcher, str_warn, str_info, print_table, shorten_metric_name
+from concurrent.futures import ThreadPoolExecutor
+
+from testutil import getStdout, str_warn, str_info, print_table, shorten_metric_name
 from testglobals import getConfig, ghc_env, getTestRun, TestConfig, \
                         TestOptions, brokens, PerfMetric
 from my_typing import TestName
@@ -151,7 +153,6 @@ config.broken_tests |= {TestName(t) for t in args.broken_test}
 
 if args.threads:
     config.threads = args.threads
-    config.use_threads = True
 
 if args.verbose is not None:
     config.verbose = args.verbose
@@ -481,26 +482,28 @@ if config.list_broken:
         print('WARNING:', len(t.framework_failures), 'framework failures!')
         print('')
 else:
-    # completion watcher
-    watcher = Watcher(len(parallelTests))
-
     # Now run all the tests
     try:
-        for oneTest in parallelTests:
-            if stopping():
-                break
-            oneTest(watcher)
+        with ThreadPoolExecutor(max_workers=config.threads) as executor:
+            for oneTest in parallelTests:
+                if stopping():
+                    break
+                oneTest(executor)
 
-        # wait for parallel tests to finish
-        if not stopping():
-            watcher.wait()
+            # wait for parallel tests to finish
+            if not stopping():
+                executor.shutdown(wait=True)
 
         # Run the following tests purely sequential
-        config.use_threads = False
-        for oneTest in aloneTests:
-            if stopping():
-                break
-            oneTest(watcher)
+        with ThreadPoolExecutor(max_workers=1) as executor:
+            for oneTest in aloneTests:
+                if stopping():
+                    break
+                oneTest(executor)
+
+            if not stopping():
+                executor.shutdown(wait=True)
+
     except KeyboardInterrupt:
         pass
 


=====================================
testsuite/driver/testglobals.py
=====================================
@@ -177,7 +177,6 @@ class TestConfig:
 
         # threads
         self.threads = 1
-        self.use_threads = False
 
         # tests which should be considered to be broken during this testsuite
         # run.


=====================================
testsuite/driver/testlib.py
=====================================
@@ -36,10 +36,7 @@ from my_typing import *
 from threading import Timer
 from collections import OrderedDict
 
-global pool_sema
-if config.use_threads:
-    import threading
-    pool_sema = threading.BoundedSemaphore(value=config.threads)
+import threading
 
 global wantToStop
 wantToStop = False
@@ -84,12 +81,7 @@ def get_all_ways() -> Set[WayName]:
 # testdir_testopts after each test).
 
 global testopts_local
-if config.use_threads:
-    testopts_local = threading.local()
-else:
-    class TestOpts_Local:
-        pass
-    testopts_local = TestOpts_Local() # type: ignore
+testopts_local = threading.local()
 
 def getTestOpts() -> TestOptions:
     return testopts_local.x
@@ -1020,16 +1012,8 @@ parallelTests = []
 aloneTests = []
 allTestNames = set([])  # type: Set[TestName]
 
-def runTest(watcher, opts, name: TestName, func, args):
-    if config.use_threads:
-        pool_sema.acquire()
-        t = threading.Thread(target=test_common_thread,
-                             name=name,
-                             args=(watcher, name, opts, func, args))
-        t.daemon = False
-        t.start()
-    else:
-        test_common_work(watcher, name, opts, func, args)
+def runTest(executor, opts, name: TestName, func, args):
+    return executor.submit(test_common_work, name, opts, func, args)
 
 # name  :: String
 # setup :: [TestOpt] -> IO ()
@@ -1067,20 +1051,13 @@ def test(name: TestName,
     if name in config.broken_tests:
         myTestOpts.expect = 'fail'
 
-    thisTest = lambda watcher: runTest(watcher, myTestOpts, name, func, args)
+    thisTest = lambda executor: runTest(executor, myTestOpts, name, func, args)
     if myTestOpts.alone:
         aloneTests.append(thisTest)
     else:
         parallelTests.append(thisTest)
     allTestNames.add(name)
 
-if config.use_threads:
-    def test_common_thread(watcher, name, opts, func, args):
-            try:
-                test_common_work(watcher, name, opts, func, args)
-            finally:
-                pool_sema.release()
-
 def get_package_cache_timestamp() -> float:
     if config.package_conf_cache_file is None:
         return 0.0
@@ -1094,8 +1071,7 @@ do_not_copy = ('.hi', '.o', '.dyn_hi'
               , '.dyn_o', '.out'
               ,'.hi-boot', '.o-boot') # 12112
 
-def test_common_work(watcher: testutil.Watcher,
-                     name: TestName, opts,
+def test_common_work(name: TestName, opts,
                      func, args) -> None:
     try:
         t.total_tests += 1
@@ -1214,8 +1190,6 @@ def test_common_work(watcher: testutil.Watcher,
 
     except Exception as e:
         framework_fail(name, None, 'Unhandled exception: ' + str(e))
-    finally:
-        watcher.notify()
 
 def do_test(name: TestName,
             way: WayName,


=====================================
testsuite/driver/testutil.py
=====================================
@@ -5,8 +5,6 @@ import tempfile
 from pathlib import Path, PurePath
 from term_color import Color, colored
 
-import threading
-
 from my_typing import *
 
 
@@ -125,24 +123,6 @@ else:
         else:
             os.symlink(str(src), str(dst))
 
-class Watcher(object):
-    def __init__(self, count: int) -> None:
-        self.pool = count
-        self.evt = threading.Event()
-        self.sync_lock = threading.Lock()
-        if count <= 0:
-            self.evt.set()
-
-    def wait(self):
-        self.evt.wait()
-
-    def notify(self):
-        self.sync_lock.acquire()
-        self.pool -= 1
-        if self.pool <= 0:
-            self.evt.set()
-        self.sync_lock.release()
-
 def memoize(f):
     """
     A decorator to memoize a nullary function.



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/7eac2468a726f217dd97c5e2884f6b552e8ef11d...ca6673e3cab496bbeed2ced47b40bcf1e0d0b3cd

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/7eac2468a726f217dd97c5e2884f6b552e8ef11d...ca6673e3cab496bbeed2ced47b40bcf1e0d0b3cd
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230208/adb38396/attachment-0001.html>


More information about the ghc-commits mailing list