Changeset 109264 in webkit

Timestamp:

Feb 29, 2012 2:40:32 PM (12 years ago)

Author:

dpranke@chromium.org

Message:

nrwt: support more than two drivers in DriverProxy
​https://bugs.webkit.org/show_bug.cgi?id=79736

Reviewed by Adam Barth.

Now that we can support per-test command lines for
Drivers, modify DriverProxy to keep a map of running
drivers for each needed command-line; this will allow
us to transparently maintain a pool of appropriately
configured DRTs without having to constantly start and stop
them.

Note that this potentially raises a garbage collection
problem - the number of running DRTs will grow with the
number of different sets of command line args. For now
this is no worse than the current code - if you're running
with pixel tests, you will only need one DRT per worker,
and if you aren't, you'll need two (one for text-only tests,
and one for reftests).

An alternative would be to only ever have one running driver,
and restart the driver as the command line changes, but this
might (?) slow down execution in the text-only case - we
should benchmark this because it would be simpler and possibly
allow us to eliminate DriverProxy altogether.

• Scripts/webkitpy/layout_tests/port/driver.py:

(DriverProxy.init):
(DriverProxy):
(DriverProxy._make_driver):
(DriverProxy.run_test):
(DriverProxy.has_crashed):
(DriverProxy.stop):
(DriverProxy.cmd_line):
(DriverProxy._cmd_line_as_key):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/driver.py (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-02-29  Dirk Pranke  <dpranke@chromium.org>
+
+        nrwt: support more than two drivers in DriverProxy
+        https://bugs.webkit.org/show_bug.cgi?id=79736
+
+        Reviewed by Adam Barth.
+
+        Now that we can support per-test command lines for
+        Drivers, modify DriverProxy to keep a map of running
+        drivers for each needed command-line; this will allow
+        us to transparently maintain a pool of appropriately
+        configured DRTs without having to constantly start and stop
+        them.
+
+        Note that this potentially raises a garbage collection
+        problem - the number of running DRTs will grow with the
+        number of different sets of command line args. For now
+        this is no worse than the current code - if you're running
+        with pixel tests, you will only need one DRT per worker,
+        and if you aren't, you'll need two (one for text-only tests,
+        and one for reftests).
+
+        An alternative would be to only ever have one running driver,
+        and restart the driver as the command line changes, but this
+        might (?) slow down execution in the text-only case - we
+        should benchmark this because it would be simpler and possibly
+        allow us to eliminate DriverProxy altogether.
+
+        * Scripts/webkitpy/layout_tests/port/driver.py:
+        (DriverProxy.__init__):
+        (DriverProxy):
+        (DriverProxy._make_driver):
+        (DriverProxy.run_test):
+        (DriverProxy.has_crashed):
+        (DriverProxy.stop):
+        (DriverProxy.cmd_line):
+        (DriverProxy._cmd_line_as_key):
+
 2012-02-29  Adrienne Walker  <enne@google.com>
 

trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py

@@ -173 +173 @@
 
     def __init__(self, port, worker_number, driver_instance_constructor, pixel_tests, no_timeout):
+        self._port = port
+        self._worker_number = worker_number
+        self._driver_instance_constructor = driver_instance_constructor
+        self._no_timeout = no_timeout
         self._pixel_tests = pixel_tests
-        self._driver = driver_instance_constructor(port, worker_number, pixel_tests, no_timeout)
-        if pixel_tests:
-            self._reftest_driver = self._driver
-        else:
-            self._reftest_driver = driver_instance_constructor(port, worker_number, True, no_timeout)
-
+
+        # FIXME: We shouldn't need to create a driver until we actually run a test.
+        self._driver = self._make_driver(pixel_tests)
+        self._running_drivers = {}
+        self._running_drivers[self._cmd_line_as_key(pixel_tests, [])] = self._driver
+
+    def _make_driver(self, pixel_tests):
+        return self._driver_instance_constructor(self._port, self._worker_number, pixel_tests, self._no_timeout)
+
+    # FIXME: this should be a @classmethod (or implemented on Port instead).
     def is_http_test(self, test_name):
         return self._driver.is_http_test(test_name)
 
+    # FIXME: this should be a @classmethod (or implemented on Port instead).
     def test_to_uri(self, test_name):
         return self._driver.test_to_uri(test_name)
 
+    # FIXME: this should be a @classmethod (or implemented on Port instead).
     def uri_to_test(self, uri):
         return self._driver.uri_to_test(uri)
 
     def run_test(self, driver_input):
-        if driver_input.is_reftest:
-            return self._reftest_driver.run_test(driver_input)
-        return self._driver.run_test(driver_input)
-
-    def has_crashed(self):
-        return self._driver.has_crashed() or self._reftest_driver.has_crashed()
+        pixel_tests_needed = self._pixel_tests or driver_input.is_reftest
+        cmd_line_key = self._cmd_line_as_key(pixel_tests_needed, [])
+        if not cmd_line_key in self._running_drivers:
+            self._running_drivers[cmd_line_key] = self._make_driver(pixel_tests_needed)
+
+        return self._running_drivers[cmd_line_key].run_test(driver_input)
 
     def start(self):
@@ -207 +217 @@
         self._driver.start(self._pixel_tests, [])
 
+    def has_crashed(self):
+        return any(driver.has_crashed() for driver in self._running_drivers.values())
+
     def stop(self):
-        self._driver.stop()
-        self._reftest_driver.stop()
-
-    def cmd_line(self, pixel_tests, per_test_args):
-        cmd_line = self._driver.cmd_line(pixel_tests, per_test_args)
-        if self._driver != self._reftest_driver:
-            cmd_line += ['; '] + self._reftest_driver.cmd_line(pixel_tests, per_test_args)
-        return cmd_line
+        for driver in self._running_drivers.values():
+            driver.stop()
+
+    # FIXME: this should be a @classmethod (or implemented on Port instead).
+    def cmd_line(self, pixel_tests=None, per_test_args=None):
+        return self._driver.cmd_line(pixel_tests or self._pixel_tests, per_test_args or [])
+
+    def _cmd_line_as_key(self, pixel_tests, per_test_args):
+        return ' '.join(self.cmd_line(pixel_tests, per_test_args))