Title: [109264] trunk/Tools
Revision
109264
Author
dpra...@chromium.org
Date
2012-02-29 14:40:32 -0800 (Wed, 29 Feb 2012)

Log 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):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (109263 => 109264)


--- trunk/Tools/ChangeLog	2012-02-29 22:35:01 UTC (rev 109263)
+++ trunk/Tools/ChangeLog	2012-02-29 22:40:32 UTC (rev 109264)
@@ -1,3 +1,41 @@
+2012-02-29  Dirk Pranke  <dpra...@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  <e...@google.com>
 
         Unreviewed, add myself as a reviewer

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py (109263 => 109264)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py	2012-02-29 22:35:01 UTC (rev 109263)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py	2012-02-29 22:40:32 UTC (rev 109264)
@@ -172,29 +172,39 @@
     single driver."""
 
     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)
+        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)
 
-    def has_crashed(self):
-        return self._driver.has_crashed() or self._reftest_driver.has_crashed()
+        return self._running_drivers[cmd_line_key].run_test(driver_input)
 
     def start(self):
         # FIXME: Callers shouldn't normally call this, since this routine
@@ -206,12 +216,16 @@
         # into run_test() directly.
         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()
+        for driver in self._running_drivers.values():
+            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
+    # 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))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to