Diff
Modified: trunk/Tools/ChangeLog (90190 => 90191)
--- trunk/Tools/ChangeLog 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/ChangeLog 2011-07-01 01:55:08 UTC (rev 90191)
@@ -1,5 +1,28 @@
2011-06-30 Adam Barth <[email protected]>
+ Reviewed by Eric Seidel.
+
+ Remove threaded mode from new-run-webkit-tests
+ https://bugs.webkit.org/show_bug.cgi?id=63771
+
+ This mode is not used and is buggy. Rather than carry around a bunch
+ of unused buggy code, we should rip it out and focus on the
+ multiprocess implementation.
+
+ * Scripts/webkitpy/common/system/executive.py:
+ * Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker.py:
+ * Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py:
+ * Scripts/webkitpy/layout_tests/layout_package/worker.py:
+ * Scripts/webkitpy/layout_tests/port/base.py:
+ * Scripts/webkitpy/layout_tests/port/chromium.py:
+ * Scripts/webkitpy/layout_tests/port/mac.py:
+ * Scripts/webkitpy/layout_tests/port/port_testcase.py:
+ * Scripts/webkitpy/layout_tests/port/server_process.py:
+ * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+
+2011-06-30 Adam Barth <[email protected]>
+
Reviewed by Darin Adler.
Clean up output from new-run-webkit-tests
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -28,16 +28,14 @@
"""Module for handling messages and concurrency for run-webkit-tests.
-This module implements a message broker that connects the manager
-(TestRunner2) to the workers: it provides a messaging abstraction and
-message loops (building on top of message_broker), and handles starting
-workers by launching threads and/or processes depending on the
-requested configuration.
+This module implements a message broker that connects the manager to the
+workers: it provides a messaging abstraction and message loops (building on
+top of message_broker), and handles starting workers by launching processes.
There are a lot of classes and objects involved in a fully connected system.
They interact more or less like:
-TestRunner2 --> _InlineManager ---> _InlineWorker <-> Worker
+ Manager --> _InlineManager ---> _InlineWorker <-> Worker
^ \ / ^
| v v |
\-------------------- MessageBroker -------------/
@@ -80,7 +78,7 @@
options = [
optparse.make_option("--worker-model", action=""
help=("controls worker model. Valid values are "
- "'inline', 'threads', and 'processes'.")),
+ "'inline' and 'processes'.")),
]
return options
@@ -103,9 +101,6 @@
if worker_model == 'inline':
queue_class = Queue.Queue
manager_class = _InlineManager
- elif worker_model == 'threads':
- queue_class = Queue.Queue
- manager_class = _ThreadedManager
elif worker_model == 'processes' and multiprocessing:
queue_class = multiprocessing.Queue
manager_class = _MultiProcessManager
@@ -182,18 +177,6 @@
self._broker.run_all_pending(MANAGER_TOPIC, self._client)
-class _ThreadedManager(_ManagerConnection):
- def __init__(self, broker, port, options, client, worker_class):
- _ManagerConnection.__init__(self, broker, options, client, worker_class)
- self._port = port
-
- def start_worker(self, worker_number):
- worker_connection = _ThreadedWorkerConnection(self._broker, self._port,
- self._worker_class, worker_number)
- worker_connection.start()
- return worker_connection
-
-
class _MultiProcessManager(_ManagerConnection):
def __init__(self, broker, port, options, client, worker_class):
# Note that this class does not keep a handle to the actual port
@@ -261,48 +244,6 @@
self._broker.run_all_pending(MANAGER_TOPIC, self._manager_client)
-class _Thread(threading.Thread):
- def __init__(self, worker_connection, port, client):
- threading.Thread.__init__(self)
- self._worker_connection = worker_connection
- self._port = port
- self._client = client
-
- def cancel(self):
- return self._client.cancel()
-
- def log_wedged_worker(self, test_name):
- stack_utils.log_thread_state(_log.error, self._client.name(), self.ident, " is wedged on test %s" % test_name)
-
- def run(self):
- # FIXME: We can remove this once everyone is on 2.6.
- if not hasattr(self, 'ident'):
- self.ident = thread.get_ident()
- self._client.run(self._port)
-
-
-class _ThreadedWorkerConnection(_WorkerConnection):
- def __init__(self, broker, port, worker_class, worker_number):
- _WorkerConnection.__init__(self, broker, worker_class, worker_number, port._options)
- self._thread = _Thread(self, port, self._client)
-
- def cancel(self):
- return self._thread.cancel()
-
- def is_alive(self):
- # FIXME: Change this to is_alive once everyone is on 2.6.
- return self._thread.isAlive()
-
- def join(self, timeout):
- return self._thread.join(timeout)
-
- def log_wedged_worker(self, test_name):
- return self._thread.log_wedged_worker(test_name)
-
- def start(self):
- self._thread.start()
-
-
if multiprocessing:
class _Process(multiprocessing.Process):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unittest.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -119,9 +119,6 @@
def test_get__inline(self):
self.assertTrue(make_broker(self, 'inline') is not None)
- def test_get__threads(self):
- self.assertTrue(make_broker(self, 'threads') is not None)
-
def test_get__processes(self):
# This test sometimes fails on Windows. See <http://webkit.org/b/55087>.
if sys.platform in ('cygwin', 'win32'):
@@ -245,15 +242,6 @@
return multiprocessing.Queue()
-class ThreadedBrokerTests(_TestsMixin, unittest.TestCase):
- def setUp(self):
- _TestsMixin.setUp(self)
- self._worker_model = 'threads'
-
- def queue(self):
- return Queue.Queue()
-
-
class FunctionsTest(unittest.TestCase):
def test_runtime_options(self):
option_list = manager_worker_broker.runtime_options()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -249,8 +249,6 @@
Returns: a TestResult object.
"""
- # poll() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
if not self._driver or self._driver.poll() is not None:
self._driver = self._port.create_driver(self._worker_number)
self._driver.start()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -155,7 +155,7 @@
def default_worker_model(self):
if self._multiprocessing_is_available:
return 'processes'
- return 'threads'
+ return 'inline'
def baseline_path(self):
"""Return the absolute path to the directory to store new baselines in for this port."""
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -246,8 +246,6 @@
_log.debug("Stopping layout test helper")
self._helper.stdin.write("x\n")
self._helper.stdin.close()
- # wait() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
self._helper.wait()
def all_baseline_variants(self):
@@ -410,8 +408,6 @@
close_fds=close_flag)
def poll(self):
- # poll() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
return self._proc.poll()
def _write_command_and_read_line(self, input=None):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/mac.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/mac.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/mac.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -86,15 +86,6 @@
if not hasattr(self._options, 'time-out-ms') or self._options.time_out_ms is None:
self._options.time_out_ms = 35000
- def default_child_processes(self):
- # FIXME: new-run-webkit-tests is unstable on Mac running more than
- # four threads in parallel.
- # See https://bugs.webkit.org/show_bug.cgi?id=36622
- child_processes = WebKitPort.default_child_processes(self)
- if not self._multiprocessing_is_available and child_processes > 4:
- return 4
- return child_processes
-
def baseline_search_path(self):
return map(self._webkit_baseline_path, self.FALLBACK_PATHS[self._version])
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -75,7 +75,7 @@
if multiprocessing:
self.assertEqual(port.default_worker_model(), 'processes')
else:
- self.assertEqual(port.default_worker_model(), 'threads')
+ self.assertEqual(port.default_worker_model(), 'inline')
def test_driver_cmd_line(self):
port = self.make_port()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -105,8 +105,6 @@
"""Check to see if the underlying process is running; returns None
if it still is (wrapper around subprocess.poll)."""
if self._proc:
- # poll() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
return self._proc.poll()
return None
@@ -160,8 +158,6 @@
return self._read(timeout, size)
def _check_for_crash(self):
- # poll() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
if self._proc.poll() != None:
self.crashed = True
self.handle_interrupt()
@@ -251,12 +247,8 @@
# force-kill the process if necessary.
KILL_TIMEOUT = 3.0
timeout = time.time() + KILL_TIMEOUT
- # poll() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
while self._proc.poll() is None and time.time() < timeout:
time.sleep(0.1)
- # poll() is not threadsafe and can throw OSError due to:
- # http://bugs.python.org/issue1731717
if self._proc.poll() is None:
_log.warning('stopping %s timed out, killing it' %
self._name)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -401,7 +401,7 @@
# FIXME: Display default number of child processes that will run.
optparse.make_option("--worker-model", action=""
default=None, help=("controls worker model. Valid values are "
- "'inline', 'threads', and 'processes'.")),
+ "'inline' and 'processes'.")),
optparse.make_option("-f", "--experimental-fully-parallel",
action="" default=False,
help="run all tests in parallel"),
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (90190 => 90191)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-07-01 01:33:54 UTC (rev 90190)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-07-01 01:55:08 UTC (rev 90191)
@@ -204,17 +204,17 @@
def test_child_process_1(self):
_, _, regular_output, _ = logging_run(
- ['--print', 'config', '--worker-model', 'threads', '--child-processes', '1'])
+ ['--print', 'config', '--worker-model', 'processes', '--child-processes', '1'])
self.assertTrue(any(['Running 1 ' in line for line in regular_output.get()]))
def test_child_processes_2(self):
_, _, regular_output, _ = logging_run(
- ['--print', 'config', '--worker-model', 'threads', '--child-processes', '2'])
+ ['--print', 'config', '--worker-model', 'processes', '--child-processes', '2'])
self.assertTrue(any(['Running 2 ' in line for line in regular_output.get()]))
def test_child_processes_min(self):
_, _, regular_output, _ = logging_run(
- ['--print', 'config', '--worker-model', 'threads', '--child-processes', '2', 'passes'],
+ ['--print', 'config', '--worker-model', 'processes', '--child-processes', '2', 'passes'],
tests_included=True)
self.assertTrue(any(['Running 1 ' in line for line in regular_output.get()]))
@@ -611,9 +611,6 @@
if multiprocessing and sys.platform not in ('cygwin', 'win32'):
self.assertTrue(passing_run(['--worker-model', 'processes', '--dry-run']))
- def test_worker_model__threads(self):
- self.assertTrue(passing_run(['--worker-model', 'threads']))
-
def test_worker_model__unknown(self):
self.assertRaises(ValueError, logging_run,
['--worker-model', 'unknown'])
