Speed up parsing of TestExpectations

Currently the parsing step of TestExpectations takes ~11 seconds on a
development machine, with the majority of that time in the
_collect_matching_tests() step, building the list of |matching_tests|.

This can be sped up considerably by changing _all_tests from a flat
list of test names to a dictionary tree of path components. Then
matching a directory of tests is a search in the tree + iterating the
subtree below the matching directory node, instead of a search through
the full list with substring matching.

This reduces the time locally for parsing TestExpectations from ~11 to
~3 seconds, saving ~72% of the time on this step.

With logging in web_tests/controllers/manager.py:

+        _log.debug("> build TestExpectations");
+        start = time.time()
         self._expectations = test_expectations.TestExpectations(self._port, test_names)
+        _log.debug("< build TestExpectations %fs", time.time()-start);

Before:
15:28:22.789 167015 < build TestExpectations 11.192820s

After:
11:23:38.824 11280 < build TestExpectations 3.105809s

On the windows try bot:

Before: https://chromium-swarm.appspot.com/task?id=4874d0c111ee7a10
11:48:32.359 11980 Parsing expectations ...
11:49:15.140 11980 Found 3676 tests (total 86570); running 3439, skipping 237.
= 43 seconds

After: https://chromium-swarm.appspot.com/task?id=4874f28bff322210
12:24:46.461 11648 Parsing expectations ...
12:25:22.805 11648 Found 3676 tests (total 86570); running 3439, skipping 237.
= 36 seconds

R=robertma@chromium.org

Bug: 982208
Change-Id: I04f5c3c1b09cf4f1b42ad501d624f83f6f378a97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906472Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714530}

third_party/blink/tools/blinkpy/w3c/wpt_metadata_builder_unittest.py

@@ -14,20 +14,29 @@ from blinkpy.w3c.wpt_manifest import BASE_MANIFEST_NAME
 from blinkpy.w3c.wpt_metadata_builder import WPTMetadataBuilder
 
 
-def _make_expectation(port, test_name, test_statuses):
-    """Creates an expectation object for a single test.
+def _make_expectation(port, test_path, test_statuses, test_names=[]):
+    """Creates an expectation object for a single test or directory.
 
     Args:
         port: the port to run against
-        test_name: the name of the test
+        test_path: the path to set expectations for
         test_status: the statuses of the test
+        test_names: a set of tests under the 'test_path'. Should be non-empty
+            when the 'test_path' is a directory instead of a single test
 
     Returns:
         An expectation object with the given test and statuses.
     """
     expectation_dict = OrderedDict()
-    expectation_dict["expectations"] = "Bug(test) %s [ %s ]" % (test_name, test_statuses)
-    return TestExpectations(port, tests=[test_name], expectations_dict=expectation_dict)
+    expectation_dict["expectations"] = "Bug(test) %s [ %s ]" % (test_path, test_statuses)
+
+    # When test_path is a dir, we expect test_names to be provided.
+    is_dir = test_path.endswith('/')
+    assert is_dir == bool(test_names)
+    if not is_dir:
+        test_names = [test_path]
+
+    return TestExpectations(port, tests=test_names, expectations_dict=expectation_dict)
 
 
 class WPTMetadataBuilderTest(unittest.TestCase):
@@ -86,10 +95,11 @@ class WPTMetadataBuilderTest(unittest.TestCase):
 
     def test_skipped_directory(self):
         """A skipped WPT directory should get a dir-wide metadata file."""
-        test_name = "external/wpt/test_dir/"
-        expectations = _make_expectation(self.port, test_name, "SKIP")
+        test_dir = "external/wpt/test_dir/"
+        test_name = "external/wpt/test_dir/test.html"
+        expectations = _make_expectation(self.port, test_dir, "SKIP", test_names=[test_name])
         metadata_builder = WPTMetadataBuilder(expectations, self.port)
-        filename, contents = metadata_builder.get_metadata_filename_and_contents(test_name, 'SKIP')
+        filename, contents = metadata_builder.get_metadata_filename_and_contents(test_dir, 'SKIP')
         self.assertEqual(os.path.join("test_dir", "__dir__.ini"), filename)
         self.assertEqual("disabled: wpt_metadata_builder.py\n", contents)
 

third_party/blink/tools/blinkpy/web_tests/models/test_expectations.py

@@ -84,6 +84,77 @@ _SPECIFIER_GROUPS = [
     set(s.upper() for s in _BUILD_TYPE_TOKEN_LIST)
 ]
 
+
+class AllTests(object):
+    """A class to query for path-prefix matches against the list of all tests."""
+    def __init__(self, list_of_tests):
+        self._tree = dict()
+        for test in list_of_tests:
+            assert not test.startswith('/')
+            assert not test.endswith('/')
+            assert test.replace('//', '/') == test
+            AllTests._add_path_to_tree(self._tree, test.split('/'))
+
+    def find_matching_tests(self, path_prefix):
+        assert not path_prefix.startswith('/')
+        assert path_prefix.replace('//', '/') == path_prefix
+
+        subtree = AllTests._find_subtree_with_prefix(self._tree, path_prefix.rstrip('/').split('/'))
+        if subtree is None:
+            # No match found.
+            return []
+        if not subtree:
+            # We found a leaf node, an exact match on |path_prefix|.
+            return [path_prefix]
+        return AllTests._all_paths_under_subtree(subtree, path_prefix)
+
+    @staticmethod
+    def _find_subtree_with_prefix(subtree, path_list):
+        # Reached the end of the path, we matched to this point in the
+        # dictionary tree.
+        if not path_list:
+            return subtree
+        path_component = path_list[0]
+        # A component of the path does not exist in the dictionary tree.
+        if not path_component in subtree:
+            return None
+        path_remainder = path_list[1:]
+        return AllTests._find_subtree_with_prefix(subtree[path_component], path_remainder)
+
+    @staticmethod
+    def _all_paths_under_subtree(subtree, prefix):
+        if not subtree:
+            return []
+
+        if prefix and not prefix.endswith('/'):
+            prefix = prefix + '/'
+
+        paths = []
+        for child_path, child_tree in subtree.iteritems():
+            if not child_tree:
+                paths.append(prefix + child_path)
+            else:
+                paths.extend(AllTests._all_paths_under_subtree(child_tree, prefix + child_path))
+        return paths
+
+    @staticmethod
+    def _add_path_to_tree(subtree, path_list):
+        # When |path_list| is empty, we reached the end of the path,
+        # so don't add anything more to |subtree|.
+        if not path_list:
+            # If subtree is not empty, then we have the same path listed
+            # twice in the initial list of tests.
+            assert len(subtree) == 0
+            return
+
+        path_component = path_list[0]
+        if not path_component in subtree:
+            subtree[path_component] = dict()
+
+        path_remainder = path_list[1:]
+        AllTests._add_path_to_tree(subtree[path_component], path_remainder)
+
+
 class TestExpectationParser(object):
     """Provides parsing facilities for lines in the test_expectation.txt file."""
 
@@ -101,12 +172,7 @@ class TestExpectationParser(object):
         self._port = port
         self._test_configuration_converter = TestConfigurationConverter(
             set(port.all_test_configurations()), port.configuration_specifier_macros())
-
-        if all_tests:
-            self._all_tests = set(all_tests)
-        else:
-            self._all_tests = set()
-
+        self._all_tests = AllTests(all_tests) if all_tests else None
         self._is_lint_mode = is_lint_mode
 
     def parse(self, filename, expectations_string):
@@ -220,16 +286,7 @@ class TestExpectationParser(object):
             expectation_line.matching_tests = [expectation_line.path]
             return
 
-        if not expectation_line.is_file:
-            # this is a test category, return all the tests of the category.
-            expectation_line.matching_tests = [test for test in self._all_tests if test.startswith(expectation_line.path)]
-            return
-
-        # this is a test file, do a quick check if it's in the
-        # full test suite.
-        if expectation_line.path in self._all_tests:
-            expectation_line.matching_tests.append(expectation_line.path)
-
+        expectation_line.matching_tests = self._all_tests.find_matching_tests(expectation_line.path)
 
 class TestExpectationLine(object):
     """Represents a line in test expectations file."""