commit f77ced70a0cf6fef78ade47aac57a3ba1ed8daa7
author twigg <twigg@google.com> Tue Sep 01 16:44:53 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Sep 01 16:45:50 2020 -0700
tree 99e7db691fd262241819b6bdbcd31bad82e0a53d
parent 96898592c13228b6788ca6f289e291e709c60a2a

Cache the invocation of TestTrimmingTransition.applyTransition.

This implementation uses the pre-existing BuildOptionsCache class inside TestTrimmingTransition to resolve the performance issue with --trim_test_configuration on longwide. (Previous attempts used a SkyFunction, a custom cache at ConfigurationResolve, or a custom cache at TestTrimmingTransition.)

PiperOrigin-RevId: 329607319

src/main/java/com/google/devtools/build/lib/analysis/BUILD

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 1a26394..f367071 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -2270,6 +2270,7 @@
     srcs = ["test/TestTrimmingTransitionFactory.java"],
     deps = [
         ":config/build_options",
+        ":config/build_options_cache",
         ":config/fragment_options",
         ":config/transitions/no_transition",
         ":config/transitions/patch_transition",

src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
index a759b25..3faac3c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
@@ -15,6 +15,7 @@
 
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptionsCache;
 import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
@@ -42,6 +43,14 @@
   public enum TestTrimmingTransition implements PatchTransition {
     INSTANCE;
 
+    // This cache is to prevent major slowdowns when using --trim_test_configuration. This
+    // transition is always invoked on every target in the top-level invocation. Thus, a wide
+    // invocation, like //..., will cause the transition to be invoked on a large number of targets
+    // leading to significant performance degradation. (Notably, the transition itself is somewhat
+    // fast; however, the post-processing of the BuildOptions results into a BuildConfiguration
+    // takes a significant amount of time).
+    private static final BuildOptionsCache<Integer> cache = new BuildOptionsCache<>();
+
     @Override
     public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
       return ImmutableSet.of(TestOptions.class);
@@ -58,9 +67,15 @@
         // nothing to do, trimming is disabled
         return originalOptions.underlying();
       }
-      return originalOptions.underlying().toBuilder()
-          .removeFragmentOptions(TestOptions.class)
-          .build();
+      return cache.applyTransition(
+          originalOptions,
+          // The transition uses no non-BuildOptions arguments
+          0,
+          () -> {
+            return originalOptions.underlying().toBuilder()
+                .removeFragmentOptions(TestOptions.class)
+                .build();
+          });
     }
   }
 
@@ -75,7 +90,7 @@
     }
 
     Set<String> referencedTestOptions =
-        new LinkedHashSet<String>(ruleClass.getOptionReferenceFunction().apply(rule));
+        new LinkedHashSet<>(ruleClass.getOptionReferenceFunction().apply(rule));
     referencedTestOptions.retainAll(TEST_OPTIONS);
     if (!referencedTestOptions.isEmpty()) {
       // Test-option-referencing config_setting; no need to trim here.