aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
diff options
context:
space:
mode:
authorGravatar mstaib <mstaib@google.com>2018-06-18 13:37:49 -0700
committerGravatar Copybara-Service <copybara-piper@google.com>2018-06-18 13:39:20 -0700
commita9b55855b15ca04b8aeb3917a31d997169701ba9 (patch)
treef3da8585ca8804d99bc9fa9e92e6ac9dc771d96e /src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
parent236fdeb46034fb58f6f39084e9d81e41a4693e01 (diff)
Let ConfiguredRuleClassProvider decide whether to drop the analysis cache.
ConfiguredRuleClassProvider can specify a predicate which accepts an OptionsDiff and the new BuildOptions in order to make a decision on whether the given diff requires the analysis cache to be dropped. This predicate is only called if all of the following hold: * there was an old configuration collection (if not, the cache is not dropped because there wasn't one yet) * the old configuration collection is not exactly equal to the new configuration collection (if it is, the cache is not dropped because it definitely hasn't changed) * the old configuration collection has the same number of configurations as the new collection (if not, the cache is always dropped because experimental_multi_cpu has changed, definitely not a flag which should cause old analysis cache to stick around!) If all of these hold, the old target configurations are paired up with the new target configurations by index in the configuration collection, and each pair is diffed. The predicate is called with each diff and the corresponding new configuration's build options. If any of these invocations returns true, the cache is dropped. Otherwise, the cache is kept for the next build. No implementation of this predicate is actually supplied for this change, so the old behavior (always drop the cache if the configuration changes at all) holds. RELNOTES: None. PiperOrigin-RevId: 201050049
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java')
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java39
1 files changed, 37 insertions, 2 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index bf14785ca5..acb7c4a064 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -43,6 +43,8 @@ import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiff;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.FragmentClassSet;
import com.google.devtools.build.lib.buildeventstream.BuildEventId;
@@ -146,6 +148,40 @@ public final class SkyframeBuildView {
return factory;
}
+ private boolean areConfigurationsDifferent(BuildConfigurationCollection configurations) {
+ if (this.configurations == null) {
+ // no configurations currently, no need to drop anything
+ return false;
+ }
+ if (configurations.equals(this.configurations)) {
+ // exact same configurations, no need to drop anything
+ return false;
+ }
+ if (configurations.getTargetConfigurations().size()
+ != this.configurations.getTargetConfigurations().size()) {
+ // some option that changes the number of configurations has changed, that's definitely not
+ // any of the options that are okay to change
+ return true;
+ }
+ // Here we assume that the configurations will appear in the same order between invocations -
+ // which is the case today, because the only way to have multiple configurations is to use
+ // --experimental_multi_cpu, which creates configurations in sorted order of cpu value.
+ // Those configurations are all identical except for their cpu values, so the configuration we
+ // compare against only matters for making sure the cpu matches. Which we do care about.
+ for (int configIndex = 0;
+ configIndex < configurations.getTargetConfigurations().size();
+ configIndex += 1) {
+ BuildConfiguration oldConfig = this.configurations.getTargetConfigurations().get(configIndex);
+ BuildConfiguration newConfig = configurations.getTargetConfigurations().get(configIndex);
+ OptionsDiff diff = BuildOptions.diff(oldConfig.getOptions(), newConfig.getOptions());
+ if (ruleClassProvider.shouldInvalidateCacheForDiff(diff, newConfig.getOptions())) {
+ return true;
+ }
+ }
+ // We don't need to check the host configuration because it's derived from the target options.
+ return false;
+ }
+
/** Sets the configurations. Not thread-safe. DO NOT CALL except from tests! */
@VisibleForTesting
public void setConfigurations(
@@ -153,8 +189,7 @@ public final class SkyframeBuildView {
// Clear all cached ConfiguredTargets on configuration change of if --discard_analysis_cache
// was set on the previous build. In the former case, it's not required for correctness, but
// prevents unbounded memory usage.
- if ((this.configurations != null && !configurations.equals(this.configurations))
- || skyframeAnalysisWasDiscarded) {
+ if (this.areConfigurationsDifferent(configurations) || skyframeAnalysisWasDiscarded) {
eventHandler.handle(Event.info("Build options have changed, discarding analysis cache."));
skyframeExecutor.handleConfiguredTargetChange();
}