Changeset 72641 in webkit


Ignore:
Timestamp:
Nov 23, 2010 4:15:16 PM (13 years ago)
Author:
dpranke@chromium.org
Message:

2010-11-23 Dirk Pranke <dpranke@chromium.org>

Reviewed by Tony Chang.

This patch cleans up the logic used to shard tests into groups a
bit and adds the --worker-model flag to NRWT. The flag is only
used at the moment to control whether to run single-threaded or
not, but eventually will also allow toggling between threads and
processes.

Also add a minor cleanup with _test_is_slow(), which just
eliminates some repetition and gives slightly better encapsulation.

https://bugs.webkit.org/show_bug.cgi?id=49773

  • Scripts/webkitpy/layout_tests/run_webkit_tests.py:
  • Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
Location:
trunk/WebKitTools
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r72640 r72641  
     12010-11-23  Dirk Pranke  <dpranke@chromium.org>
     2
     3        Reviewed by Tony Chang.
     4
     5        This patch cleans up the logic used to shard tests into groups a
     6        bit and adds the --worker-model flag to NRWT. The flag is only
     7        used at the moment to control whether to run single-threaded or
     8        not, but eventually will also allow toggling between threads and
     9        processes.
     10
     11        Also add a minor cleanup with _test_is_slow(), which just
     12        eliminates some repetition and gives slightly better encapsulation.
     13
     14        https://bugs.webkit.org/show_bug.cgi?id=49773
     15
     16        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
     17        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
     18
    1192010-11-23  Mihai Parparita  <mihaip@chromium.org>
    220
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

    r72551 r72641  
    486486        is used for looking up the timeout value (in ms) to use for the given
    487487        test."""
    488         if self._expectations.has_modifier(test_file, test_expectations.SLOW):
     488        if self._test_is_slow(test_file):
    489489            return TestInput(test_file, self._options.slow_time_out_ms)
    490490        return TestInput(test_file, self._options.time_out_ms)
     
    496496        return 'http' in split_path or 'websocket' in split_path
    497497
    498     def _get_test_file_queue(self, test_files):
    499         """Create the thread safe queue of lists of (test filenames, test URIs)
    500         tuples. Each TestShellThread pulls a list from this queue and runs
    501         those tests in order before grabbing the next available list.
    502 
    503         Shard the lists by directory. This helps ensure that tests that depend
    504         on each other (aka bad tests!) continue to run together as most
    505         cross-tests dependencies tend to occur within the same directory.
     498    def _test_is_slow(self, test_file):
     499        return self._expectations.has_modifier(test_file,
     500                                               test_expectations.SLOW)
     501
     502    def _shard_tests(self, test_files, use_real_shards):
     503        """Groups tests into batches.
     504        This helps ensure that tests that depend on each other (aka bad tests!)
     505        continue to run together as most cross-tests dependencies tend to
     506        occur within the same directory. If use_real_shards is false, we
     507        put each (non-HTTP/websocket) test into its own shard for maximum
     508        concurrency instead of trying to do any sort of real sharding.
    506509
    507510        Return:
    508           The Queue of lists of TestInput objects.
    509         """
     511            A list of lists of TestInput objects.
     512        """
     513        # FIXME: when we added http locking, we changed how this works such
     514        # that we always lump all of the HTTP threads into a single shard.
     515        # That will slow down experimental-fully-parallel, but it's unclear
     516        # what the best alternative is completely revamping how we track
     517        # when to grab the lock.
    510518
    511519        test_lists = []
    512520        tests_to_http_lock = []
    513         if (self._options.experimental_fully_parallel or
    514             self._is_single_threaded()):
     521        if not use_real_shards:
    515522            for test_file in test_files:
    516523                test_input = self._get_test_input_for_file(test_file)
     
    551558            test_lists.insert(0, ("tests_to_http_lock", tests_to_http_lock))
    552559
    553         filename_queue = Queue.Queue()
    554         for item in test_lists:
    555             filename_queue.put(item)
    556         return filename_queue
     560        return test_lists
    557561
    558562    def _contains_tests(self, subdir):
     
    569573          The list of threads.
    570574        """
    571         filename_queue = self._get_test_file_queue(test_files)
     575        num_workers = self._num_workers()
     576        test_lists = self._shard_tests(test_files,
     577            num_workers > 1 and not self._options.experimental_fully_parallel)
     578        filename_queue = Queue.Queue()
     579        for item in test_lists:
     580            filename_queue.put(item)
    572581
    573582        # Instantiate TestShellThreads and start them.
    574583        threads = []
    575         for worker_number in xrange(int(self._options.child_processes)):
     584        for worker_number in xrange(num_workers):
    576585            thread = dump_render_tree_thread.TestShellThread(self._port,
    577586                self._options, worker_number,
    578587                filename_queue, self._result_queue)
    579             if self._is_single_threaded():
     588            if num_workers > 1:
     589                thread.start()
     590            else:
    580591                thread.run_in_main_thread(self, result_summary)
    581             else:
    582                 thread.start()
    583592            threads.append(thread)
    584593
    585594        return threads
    586595
    587     def _is_single_threaded(self):
    588         """Returns whether we should run all the tests in the main thread."""
    589         return int(self._options.child_processes) == 1
     596    def _num_workers(self):
     597        return int(self._options.child_processes)
    590598
    591599    def _run_tests(self, file_list, result_summary):
     
    603611            result_summary: summary object to populate with the results
    604612        """
    605         # FIXME: We should use webkitpy.tool.grammar.pluralize here.
    606613        plural = ""
    607         if not self._is_single_threaded():
     614        if self._num_workers() > 1:
    608615            plural = "s"
    609616        self._printer.print_update('Starting %s%s ...' %
     
    925932                        self._options.slow_time_out_ms))
    926933
    927         if self._is_single_threaded():
     934        if self._num_workers() == 1:
    928935            p.print_config("Running one %s" % self._port.driver_name())
    929936        else:
     
    931938                           (self._options.child_processes,
    932939                            self._port.driver_name()))
     940        p.print_config("Worker model: %s" % self._options.worker_model)
    933941        p.print_config("")
    934942
     
    10451053            filename = test_tuple.filename
    10461054            is_timeout_crash_or_slow = False
    1047             if self._expectations.has_modifier(filename,
    1048                                                test_expectations.SLOW):
     1055            if self._test_is_slow(filename):
    10491056                is_timeout_crash_or_slow = True
    10501057                slow_tests.append(test_tuple)
     
    13611368    """Sets the options values that depend on other options values."""
    13621369
     1370    if options.worker_model == 'inline':
     1371        if options.child_processes and int(options.child_processes) > 1:
     1372            _log.warning("--worker-model=inline overrides --child-processes")
     1373        options.child_processes = "1"
    13631374    if not options.child_processes:
    1364         # FIXME: Investigate perf/flakiness impact of using cpu_count + 1.
    13651375        options.child_processes = os.environ.get("WEBKIT_TEST_CHILD_PROCESSES",
    13661376                                                 str(port_obj.default_child_processes()))
     
    15851595            help="Number of DumpRenderTrees to run in parallel."),
    15861596        # FIXME: Display default number of child processes that will run.
     1597        optparse.make_option("--worker-model", action="store",
     1598                             default="threads",
     1599                             help="controls worker model. Valid values are "
     1600                             "'inline' and 'threads' (default)."),
    15871601        optparse.make_option("--experimental-fully-parallel",
    15881602            action="store_true", default=False,
     
    16421656    options, args = option_parser.parse_args(args)
    16431657
     1658    if options.worker_model not in ('inline', 'threads'):
     1659        option_parser.error("unsupported value for --worker-model: %s" %
     1660                            options.worker_model)
     1661
    16441662    return options, args
    1645 
    16461663
    16471664
     
    16501667    port_obj = port.get(options.platform, options)
    16511668    return run(port_obj, options, args)
     1669
    16521670
    16531671if '__main__' == __name__:
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py

    r72551 r72641  
    7373    if not record_results:
    7474        args.append('--no-record-results')
     75    if not '--child-processes' in extra_args:
     76        args.extend(['--worker-model', 'inline'])
    7577    args.extend(extra_args)
    7678    if not tests_included:
     
    9395    if not '--platform' in extra_args:
    9496        args.extend(['--platform', 'test'])
     97    if not '--child-processes' in extra_args:
     98        args.extend(['--worker-model', 'inline'])
    9599    args.extend(extra_args)
    96100    if not tests_included:
     
    99103                     'websocket/tests',
    100104                     'failures/expected/*'])
    101     options, parsed_args = run_webkit_tests.parse_args(args)
    102     user = MockUser()
    103     if not port_obj:
    104         port_obj = port.get(port_name=options.platform, options=options, user=user)
    105     buildbot_output = array_stream.ArrayStream()
    106     regular_output = array_stream.ArrayStream()
    107     res = run_webkit_tests.run(port_obj, options, parsed_args,
    108                                buildbot_output=buildbot_output,
    109                                regular_output=regular_output)
     105
     106    try:
     107        oc = outputcapture.OutputCapture()
     108        oc.capture_output()
     109        options, parsed_args = run_webkit_tests.parse_args(args)
     110        user = MockUser()
     111        if not port_obj:
     112            port_obj = port.get(port_name=options.platform, options=options,
     113                                user=user)
     114        buildbot_output = array_stream.ArrayStream()
     115        regular_output = array_stream.ArrayStream()
     116        res = run_webkit_tests.run(port_obj, options, parsed_args,
     117                                   buildbot_output=buildbot_output,
     118                                   regular_output=regular_output)
     119    finally:
     120        oc.restore_output()
    110121    return (res, buildbot_output, regular_output, user)
    111122
     
    117128        '--platform', 'test',
    118129        '--no-record-results',
    119         '--child-processes', '1']
     130        '--worker-model', 'inline']
    120131    args.extend(extra_args)
    121132    if not tests_included:
     
    361372        self.assertEqual(None, test_port.tolerance_used_for_diff_image)
    362373
     374    def test_worker_model__inline(self):
     375        self.assertTrue(passing_run(['--worker-model', 'inline']))
     376
     377    def test_worker_model__threads(self):
     378        self.assertTrue(passing_run(['--worker-model', 'threads']))
     379
     380    def test_worker_model__processes(self):
     381        self.assertRaises(SystemExit, logging_run,
     382                          ['--worker-model', 'processes'])
     383
     384    def test_worker_model__unknown(self):
     385        self.assertRaises(SystemExit, logging_run,
     386                          ['--worker-model', 'unknown'])
     387
    363388MainTest = skip_if(MainTest, sys.platform == 'cygwin' and compare_version(sys, '2.6')[0] < 0, 'new-run-webkit-tests tests hang on Cygwin Python 2.5.2')
     389
    364390
    365391
     
    455481        self.assertEqual(html, expected_html)
    456482
    457     def queue_to_list(self, queue):
    458         queue_list = []
    459         while(True):
    460             try:
    461                 queue_list.append(queue.get_nowait())
    462             except Queue.Empty:
    463                 break
    464         return queue_list
    465 
    466     def test_get_test_file_queue(self):
    467         # Test that _get_test_file_queue in run_webkit_tests.TestRunner really
     483    def test_shard_tests(self):
     484        # Test that _shard_tests in run_webkit_tests.TestRunner really
    468485        # put the http tests first in the queue.
    469486        runner = TestRunnerWrapper(port=Mock(), options=Mock(), printer=Mock())
    470         runner._options.experimental_fully_parallel = False
    471487
    472488        test_list = [
     
    489505        ])
    490506
    491         runner._options.child_processes = 1
    492         test_queue_for_single_thread = runner._get_test_file_queue(test_list)
    493         runner._options.child_processes = 2
    494         test_queue_for_multi_thread = runner._get_test_file_queue(test_list)
    495 
    496         single_thread_results = self.queue_to_list(test_queue_for_single_thread)
    497         multi_thread_results = self.queue_to_list(test_queue_for_multi_thread)
     507        # FIXME: Ideally the HTTP tests don't have to all be in one shard.
     508        single_thread_results = runner._shard_tests(test_list, False)
     509        multi_thread_results = runner._shard_tests(test_list, True)
    498510
    499511        self.assertEqual("tests_to_http_lock", single_thread_results[0][0])
     
    501513        self.assertEqual("tests_to_http_lock", multi_thread_results[0][0])
    502514        self.assertEqual(expected_tests_to_http_lock, set(multi_thread_results[0][1]))
     515
    503516
    504517class DryrunTest(unittest.TestCase):
Note: See TracChangeset for help on using the changeset viewer.