ctexplain: report a build's trimmability

tools/ctexplain/BUILD

@@ -106,3 +106,16 @@ py_test(
         "//third_party/py/frozendict",
     ],
 )
+
+py_test(
+    name = "integration_test",
+    size = "small",
+    srcs = ["integration_test.py"],
+    python_version = "PY3",
+    deps = [
+        ":analyses",
+        ":bazel_api",
+        ":lib",
+        "//src/test/py/bazel:test_base",
+    ],
+)

tools/ctexplain/analyses/summary.py

@@ -13,13 +13,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Analysis that summarizes basic graph info."""
+from typing import Mapping
 from typing import Tuple
 
 # Do not edit this line. Copybara replaces it with PY2 migration helper.
 from dataclasses import dataclass
 
 from tools.ctexplain.types import ConfiguredTarget
-# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.util as util
+import tools.ctexplain.util as util
 
 
 @dataclass(frozen=True)
@@ -29,17 +30,31 @@ class _Summary():
   configurations: int
   # Number of unique target labels.
   targets: int
-  # Number of configured targets.
+  # Number of configured targets in the actual build (without trimming).
   configured_targets: int
   # Number of targets that produce multiple configured targets. This is more
   # subtle than computing configured_targets - targets. For example, if
   # targets=2 and configured_targets=4, that could mean both targets are
   # configured twice. Or it could mean the first target is configured 3 times.
   repeated_targets: int
+  # Number of configured targets if the build were optimally trimmed.
+  trimmed_configured_targets: int
 
 
-def analyze(cts: Tuple[ConfiguredTarget, ...]) -> _Summary:
-  """Runs the analysis on a build's configured targets."""
+def analyze(
+    cts: Tuple[ConfiguredTarget, ...],
+    trimmed_cts: Mapping[ConfiguredTarget, Tuple[ConfiguredTarget, ...]]
+    ) -> _Summary:
+  """Runs the analysis on a build's configured targets.
+
+  Args:
+    cts: A build's untrimmed configured targets.
+    trimmed_cts: The equivalent trimmed cts, where each map entry maps a trimmed
+      ct to the untrimmed cts that reduce to it.
+
+  Returns:
+    Analysis result as a _Summary.
+  """
   configurations = set()
   targets = set()
   label_count = {}
@@ -50,8 +65,8 @@ def analyze(cts: Tuple[ConfiguredTarget, ...]) -> _Summary:
   configured_targets = len(cts)
   repeated_targets = sum([1 for count in label_count.values() if count > 1])
 
-  return _Summary(
-      len(configurations), len(targets), configured_targets, repeated_targets)
+  return _Summary(len(configurations), len(targets), configured_targets,
+                  repeated_targets, len(trimmed_cts))
 
 
 def report(result: _Summary) -> None:
@@ -64,9 +79,15 @@ def report(result: _Summary) -> None:
     result: the analysis result
   """
   ct_surplus = util.percent_diff(result.targets, result.configured_targets)
+  trimmed_ct_surplus = util.percent_diff(result.targets,
+                                         result.trimmed_configured_targets)
+  trimming_reduction = util.percent_diff(result.configured_targets,
+                                         result.trimmed_configured_targets)
   print(f"""
 Configurations: {result.configurations}
 Targets: {result.targets}
 Configured targets: {result.configured_targets} ({ct_surplus} vs. targets)
 Targets with multiple configs: {result.repeated_targets}
+Configured targets with optimal trimming: {result.trimmed_configured_targets} ({trimmed_ct_surplus} vs. targets)
+Trimming impact on configured target graph size: {trimming_reduction}
 """)

tools/ctexplain/analyses/summary_test.py

@@ -18,15 +18,15 @@
 # Do not edit this line. Copybara replaces it with PY2 migration helper.
 from frozendict import frozendict
 
-# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.analyses.summary as summary
+import tools.ctexplain.analyses.summary as summary
 from tools.ctexplain.types import Configuration
 from tools.ctexplain.types import ConfiguredTarget
 from tools.ctexplain.types import NullConfiguration
 
 
 class SummaryTest(unittest.TestCase):
 
-  def testAnalysis(self):
+  def testAnalysisNoTrimming(self):
     config1 = Configuration(None, frozendict({'a': frozendict({'b': 'c'})}))
     config2 = Configuration(None, frozendict({'d': frozendict({'e': 'f'})}))
 
@@ -35,12 +35,34 @@ def testAnalysis(self):
     ct3 = ConfiguredTarget('//foo', NullConfiguration(), 'null', None)
     ct4 = ConfiguredTarget('//bar', config1, 'hash1', None)
 
-    res = summary.analyze((ct1, ct2, ct3, ct4))
+    untrimmed_ct_map = {ct1: (ct1,), ct2: (ct2,), ct3: (ct3,), ct4: (ct4,)}
+
+    res = summary.analyze((ct1, ct2, ct3, ct4), untrimmed_ct_map)
     self.assertEqual(3, res.configurations)
     self.assertEqual(2, res.targets)
     self.assertEqual(4, res.configured_targets)
     self.assertEqual(1, res.repeated_targets)
 
+  def testAnalysWithTrimming(self):
+    config1 = Configuration(None, frozendict({'a': frozendict({'b': 'c'})}))
+    config2 = Configuration(None, frozendict({'d': frozendict({'e': 'f'})}))
+
+    ct1 = ConfiguredTarget('//foo', config1, 'hash1', None)
+    ct2 = ConfiguredTarget('//foo', config2, 'hash2', None)
+    ct3 = ConfiguredTarget('//bar', config1, 'hash1', None)
+    ct4 = ConfiguredTarget('//bar', config2, 'hash2', None)
+
+    trimmed_ct1 = ConfiguredTarget('//foo', config1, 'trimmed_hash1', None)
+    trimmed_ct2 = ConfiguredTarget('//bar', config1, 'trimmed_hash1', None)
+    trimmed_cts = {trimmed_ct1: (ct1, ct2), trimmed_ct2: (ct3, ct4)}
+
+    res = summary.analyze((ct1, ct2, ct3, ct4), trimmed_cts)
+    self.assertEqual(2, res.configurations)
+    self.assertEqual(2, res.targets)
+    self.assertEqual(4, res.configured_targets)
+    self.assertEqual(2, res.trimmed_configured_targets)
+    self.assertEqual(2, res.repeated_targets)
+
 
 if __name__ == '__main__':
   unittest.main()

tools/ctexplain/bazel_api.py

@@ -43,7 +43,7 @@ def run_bazel_in_client(args: List[str]) -> Tuple[int, List[str], List[str]]:
     Tuple of (return code, stdout, stderr)
   """
   result = subprocess.run(
-      ["blaze"] + args,
+      ["bazel"] + args,
       cwd=os.getcwd(),
       stdout=subprocess.PIPE,
       stderr=subprocess.PIPE,
@@ -84,13 +84,17 @@ def cquery(self,
       return (False, stderr, ())
 
     cts = []
+    # TODO(gregce): cquery should return sets of targets and mostly does. But
+    # certain targets might repeat. This line was added in response to a build
+    # that showed @local_config_cc//:toolchain appearing twice.
+    seen_cts = set()
     for line in stdout:
       if not line.strip():
         continue
       ctinfo = _parse_cquery_result_line(line)
-      if ctinfo is not None:
+      if ctinfo is not None and ctinfo not in seen_cts:
         cts.append(ctinfo)
-
+        seen_cts.add(ctinfo)
     return (True, stderr, tuple(cts))
 
   def get_config(self, config_hash: str) -> Configuration:

tools/ctexplain/bazel_api_test.py

@@ -45,7 +45,7 @@ def testBasicCquery(self):
     self.assertEqual(len(cts), 1)
     self.assertEqual(cts[0].label, '//testapp:fg')
     self.assertIsNone(cts[0].config)
-    self.assertGreater(len(cts[0].config_hash), 10)
+    self.assertGreater(len(cts[0].config_hash), 6)
     self.assertIn('PlatformConfiguration', cts[0].transitive_fragments)
 
   def testFailedCquery(self):
@@ -134,7 +134,9 @@ def testConfigWithDefines(self):
 
   def testConfigWithStarlarkFlags(self):
     self.ScratchFile('testapp/defs.bzl', [
-        'def _flag_impl(settings, attr):', '  pass', 'string_flag = rule(',
+        'def _flag_impl(settings, attr):',
+        '  pass',
+        'string_flag = rule(',
         '  implementation = _flag_impl,',
         '  build_setting = config.string(flag = True)'
         ')'
@@ -144,7 +146,7 @@ def testConfigWithStarlarkFlags(self):
         'string_flag(name = "my_flag", build_setting_default = "nada")',
         'filegroup(name = "fg", srcs = ["a.file"])',
     ])
-    cquery_args = ['//testapp:fg', '--//testapp:my_flag', 'algo']
+    cquery_args = ['//testapp:fg', '--//testapp:my_flag=algo']
     cts = self._bazel_api.cquery(cquery_args)[2]
     config = self._bazel_api.get_config(cts[0].config_hash)
     user_defined_options = config.options['user-defined']

tools/ctexplain/ctexplain.py

@@ -43,11 +43,14 @@
 from absl import flags
 from dataclasses import dataclass
 
-# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.analyses.summary as summary
+# Do not edit this line. Copybara replaces it with PY2 migration helper..
+import tools.ctexplain.analyses.summary as summary
 from tools.ctexplain.bazel_api import BazelApi
-# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.lib as lib
+# Do not edit this line. Copybara replaces it with PY2 migration helper..
+import tools.ctexplain.lib as lib
 from tools.ctexplain.types import ConfiguredTarget
-# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.util as util
+# Do not edit this line. Copybara replaces it with PY2 migration helper..
+import tools.ctexplain.util as util
 
 FLAGS = flags.FLAGS
 
@@ -57,34 +60,36 @@ class Analysis():
   """Supported analysis type."""
   # The value in --analysis=<value> that triggers this analysis.
   key: str
-  # The function that invokes this analysis.
-  exec: Callable[[Tuple[ConfiguredTarget, ...]], None]
+  # The function that invokes this analysis. First parameter is the build's
+  # untrimmed configured targets. Second parameter is what the build's
+  # configured targets would be if it were perfectly trimmed.
+  exec: Callable[[Tuple[ConfiguredTarget, ...], Tuple[ConfiguredTarget, ...]], None]
   # User-friendly analysis description.
   description: str
 
 available_analyses = [
     Analysis(
         "summary",
-        lambda x: summary.report(summary.analyze(x)),
+        lambda x, y: summary.report(summary.analyze(x, y)),
         "summarizes build graph size and how trimming could help"
     ),
     Analysis(
         "culprits",
-        lambda x: print("this analysis not yet implemented"),
+        lambda x, y: print("this analysis not yet implemented"),
         "shows which flags unnecessarily fork configured targets. These\n"
         + "are conceptually mergeable."
     ),
     Analysis(
         "forked_targets",
-        lambda x: print("this analysis not yet implemented"),
+        lambda x, y: print("this analysis not yet implemented"),
         "ranks targets by how many configured targets they\n"
         + "create. These may be legitimate forks (because they behave "
         + "differently with\n different flags) or identical clones that are "
         + "conceptually mergeable."
     ),
     Analysis(
         "cloned_targets",
-        lambda x: print("this analysis not yet implemented"),
+        lambda x, y: print("this analysis not yet implemented"),
         "ranks targets by how many behavior-identical configured\n targets "
         + "they produce. These are conceptually mergeable."
     )
@@ -151,8 +156,9 @@ def main(argv):
   build_desc = ",".join(labels)
   with util.ProgressStep(f"Collecting configured targets for {build_desc}"):
     cts = lib.analyze_build(BazelApi(), labels, build_flags)
+  trimmed_cts = lib.trim_configured_targets(cts)
   for analysis in FLAGS.analysis:
-    analyses[analysis].exec(cts)
+    analyses[analysis].exec(cts, trimmed_cts)
 
 
 if __name__ == "__main__":