Fix a failure to detect action conflicts if the action conflict came about via a changed aspect that didn't change any configured targets. Realized that that failure also indicated we weren't properly accounting for aspects in our metrics. Made our metrics count them, and added in some additional fields that allow people to track the old thing if they care, or (more likely) to ease the transition from old to new: I saw 50% increases for some metrics when aspect data was added in. Also realized that we were storing a set of configured target keys during analysis for no good reason, and deleted that to save memory. The only casualty is the targets_loaded field, which was bogus anyway. unknown commit added it without anyone seeming to realize that it was not counting "loaded" targets, but rather "analyzed labels", which is not a particularly useful metric. RELNOTES[INC]: In the build event stream, BuildMetrics.TargetMetrics.targets_loaded is no longer populated. Its value was always mostly meaningless. BuildMetrics.TargetMetrics.targets_configured and BuildMetrics.ActionSummary.actions_created now include configured aspect data. PiperOrigin-RevId: 357959578
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java b/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java index 5a99018..45a4039 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java +++ b/src/main/java/com/google/devtools/build/lib/actions/AnalysisGraphStatsEvent.java
@@ -19,22 +19,24 @@ * com.google.devtools.build.lib.skyframe.SkyframeBuildView#shouldCheckForConflicts}. */ public final class AnalysisGraphStatsEvent { - private final int actionLookupValueCount; - private final int actionCount; + private final TotalAndConfiguredTargetOnlyMetric actionLookupValueCount; + private final TotalAndConfiguredTargetOnlyMetric actionCount; private final int outputArtifactCount; public AnalysisGraphStatsEvent( - int actionLookupValueCount, int actionCount, int outputArtifactCount) { + TotalAndConfiguredTargetOnlyMetric actionLookupValueCount, + TotalAndConfiguredTargetOnlyMetric actionCount, + int outputArtifactCount) { this.actionLookupValueCount = actionLookupValueCount; this.actionCount = actionCount; this.outputArtifactCount = outputArtifactCount; } - public int getActionLookupValueCount() { + public TotalAndConfiguredTargetOnlyMetric getActionLookupValueCount() { return actionLookupValueCount; } - public int getActionCount() { + public TotalAndConfiguredTargetOnlyMetric getActionCount() { return actionCount; }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 51b6dbf..da76609 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -28,6 +28,7 @@ exclude = [ "ActionInput.java", "ActionLookupKey.java", + "AnalysisGraphStatsEvent.java", "Artifact.java", "ArtifactFactory.java", "ArtifactOwner.java", @@ -136,6 +137,14 @@ ) java_library( + name = "total_and_configured_target_only_metric", + srcs = ["TotalAndConfiguredTargetOnlyMetric.java"], + deps = [ + "//third_party:auto_value", + ], +) + +java_library( name = "action_lookup_key", srcs = ["ActionLookupKey.java"], deps = [ @@ -145,6 +154,12 @@ ) java_library( + name = "analysis_graph_stats_event", + srcs = ["AnalysisGraphStatsEvent.java"], + deps = [":total_and_configured_target_only_metric"], +) + +java_library( name = "artifact_owner", srcs = ["ArtifactOwner.java"], deps = [
diff --git a/src/main/java/com/google/devtools/build/lib/actions/TotalAndConfiguredTargetOnlyMetric.java b/src/main/java/com/google/devtools/build/lib/actions/TotalAndConfiguredTargetOnlyMetric.java new file mode 100644 index 0000000..2ebd41d --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/TotalAndConfiguredTargetOnlyMetric.java
@@ -0,0 +1,34 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.actions; + +import com.google.auto.value.AutoValue; + +/** + * Class representing a metric for which we have both an overall number (including aspects) and + * specific-to-configured-target number. Usually aspects and configured targets should be considered + * together, but some historical metrics only counted configured targets, so we provide this + * granularity for consumers who care. + */ +@AutoValue +public abstract class TotalAndConfiguredTargetOnlyMetric { + public abstract int total(); + + public abstract int configuredTargetsOnly(); + + public static TotalAndConfiguredTargetOnlyMetric create(int total, int configuredTargetsOnly) { + return new AutoValue_TotalAndConfiguredTargetOnlyMetric(total, configuredTargetsOnly); + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseCompleteEvent.java index 99de301..b925e1d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseCompleteEvent.java
@@ -14,9 +14,11 @@ package com.google.devtools.build.lib.analysis; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.devtools.build.lib.pkgcache.PackageManager.PackageManagerStatistics; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric; import java.util.Collection; /** @@ -26,31 +28,23 @@ private final Collection<ConfiguredTarget> topLevelTargets; private final long timeInMs; - private int targetsLoaded; - private int targetsConfigured; + private final TotalAndConfiguredTargetOnlyMetric targetsConfigured; private final PackageManagerStatistics pkgManagerStats; - private final int actionsConstructed; + private final TotalAndConfiguredTargetOnlyMetric actionsConstructed; private final boolean analysisCacheDropped; - /** - * Construct the event. - * - * @param topLevelTargets The set of active topLevelTargets that remain. - */ public AnalysisPhaseCompleteEvent( Collection<? extends ConfiguredTarget> topLevelTargets, - int targetsLoaded, - int targetsConfigured, + TotalAndConfiguredTargetOnlyMetric targetsConfigured, + TotalAndConfiguredTargetOnlyMetric actionsConstructed, long timeInMs, PackageManagerStatistics pkgManagerStats, - int actionsConstructed, boolean analysisCacheDropped) { this.timeInMs = timeInMs; this.topLevelTargets = ImmutableList.copyOf(topLevelTargets); - this.targetsLoaded = targetsLoaded; - this.targetsConfigured = targetsConfigured; + this.targetsConfigured = checkNotNull(targetsConfigured); this.pkgManagerStats = pkgManagerStats; - this.actionsConstructed = actionsConstructed; + this.actionsConstructed = checkNotNull(actionsConstructed); this.analysisCacheDropped = analysisCacheDropped; } @@ -62,13 +56,8 @@ return topLevelTargets; } - /** Returns the number of targets loaded during analysis */ - public int getTargetsLoaded() { - return targetsLoaded; - } - - /** Returns the number of targets configured during analysis */ - public int getTargetsConfigured() { + /** Returns the number of targets/aspects configured during analysis. */ + public TotalAndConfiguredTargetOnlyMetric getTargetsConfigured() { return targetsConfigured; } @@ -76,7 +65,8 @@ return timeInMs; } - public int getActionsConstructed() { + /** Returns the actions constructed during this analysis. */ + public TotalAndConfiguredTargetOnlyMetric getActionsConstructed() { return actionsConstructed; }
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 09e4608..561b98a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -481,6 +481,7 @@ srcs = ["AnalysisPhaseCompleteEvent.java"], deps = [ ":configured_target", + "//src/main/java/com/google/devtools/build/lib/actions:total_and_configured_target_only_metric", "//src/main/java/com/google/devtools/build/lib/pkgcache", "//third_party:guava", ], @@ -633,7 +634,6 @@ "//src/main/java/com/google/devtools/build/lib/skyframe:target_pattern_phase_value", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/skyframe", - "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/protobuf:failure_details_java_proto", "//third_party:flogger", "//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index f5df8ce..239ae1e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -14,8 +14,6 @@ package com.google.devtools.build.lib.analysis; -import static java.util.stream.Collectors.toSet; - import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Supplier; @@ -37,6 +35,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.PackageRoots; +import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric; 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; @@ -86,7 +85,6 @@ import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; -import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.WalkableGraph; import java.util.ArrayList; import java.util.Collection; @@ -172,23 +170,13 @@ this.skyframeBuildView = skyframeExecutor.getSkyframeBuildView(); } - /** - * Returns two numbers: number of analyzed and number of loaded targets. - * - * <p>The first number: configured targets freshly evaluated in the last analysis run. - * - * <p>The second number: targets (not configured targets) loaded in the last analysis run. - */ - public Pair<Integer, Integer> getTargetsConfiguredAndLoaded() { - ImmutableSet<SkyKey> keys = skyframeBuildView.getEvaluatedTargetKeys(); - int targetsConfigured = keys.size(); - int targetsLoaded = - keys.stream().map(key -> ((ConfiguredTargetKey) key).getLabel()).collect(toSet()).size(); - return Pair.of(targetsConfigured, targetsLoaded); + /** Returns the number of analyzed targets/aspects. */ + public TotalAndConfiguredTargetOnlyMetric getEvaluatedCounts() { + return skyframeBuildView.getEvaluatedCounts(); } - public int getActionsConstructed() { - return skyframeBuildView.getEvaluatedActionCount(); + public TotalAndConfiguredTargetOnlyMetric getEvaluatedActionsCounts() { + return skyframeBuildView.getEvaluatedActionCounts(); } public PackageManagerStatistics getAndClearPkgManagerStatistics() { @@ -430,7 +418,7 @@ viewOptions.strictConflictChecks); setArtifactRoots(skyframeAnalysisResult.getPackageRoots()); } finally { - skyframeBuildView.clearInvalidatedConfiguredTargets(); + skyframeBuildView.clearInvalidatedActionLookupKeys(); } TopLevelConstraintSemantics topLevelConstraintSemantics =
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index efb458a..2e00138 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -745,14 +745,21 @@ message BuildMetrics { message ActionSummary { - // The total number of actions created and registered during the build. This - // includes unused actions that were constructed but not executed during - // this build. It does not include actions that were created on prior builds - // that are still valid, even if those actions had to be re-executed on this - // build. For the total number of actions that would be created if this - // invocation were "clean", see BuildGraphMetrics below. + // The total number of actions created and registered during the build, + // including both aspects and configured targets. This metric includes + // unused actions that were constructed but not executed during this build. + // It does not include actions that were created on prior builds that are + // still valid, even if those actions had to be re-executed on this build. + // For the total number of actions that would be created if this invocation + // were "clean", see BuildGraphMetrics below. int64 actions_created = 1; + // The total number of actions created this build just by configured + // targets. Used mainly to allow consumers of actions_created, which used to + // not include aspects' actions, to normalize across the Blaze release that + // switched actions_created to include all created actions. + int64 actions_created_not_including_aspects = 3; + // The total number of actions executed during the build. This includes any // remote cache hits, but excludes local action cache hits. int64 actions_executed = 2; @@ -772,15 +779,23 @@ MemoryMetrics memory_metrics = 2; message TargetMetrics { - // Number of targets loaded during this build. Does not include targets that - // were loaded on prior builds on this server and were cached. + // DEPRECATED + // No longer populated. It never measured what it was supposed to (targets + // loaded): it counted targets that were analyzed even if the underlying + // package had not changed. + // TODO(janakr): rename and remove. int64 targets_loaded = 1; - // Number of targets configured during this build. This can be greater than - // targets_loaded if the same target is configured multiple times. Does not - // include targets that were configured on prior builds on this server and + // Number of targets/aspects configured during this build. Does not include + // targets/aspects that were configured on prior builds on this server and // were cached. See BuildGraphMetrics below if you need that. int64 targets_configured = 2; + + // Number of configured targets analyzed during this build. Does not include + // aspects. Used mainly to allow consumers of targets_configured, which used + // to not include aspects, to normalize across the Blaze release that + // switched targets_configured to include aspects. + int64 targets_configured_not_including_aspects = 3; } TargetMetrics target_metrics = 3; @@ -848,10 +863,18 @@ // that were analyzed on a prior build and are still valid. May not be // populated if analysis phase was fully cached. int32 action_lookup_value_count = 1; + // How many configured targets alone were in this build: always at most + // action_lookup_value_count. Useful mainly for historical comparisons to + // TargetMetrics.targets_configured, which used to not count aspects. + int32 action_lookup_value_count_not_including_aspects = 5; // How many actions belonged to the configured targets/aspects above. It may // not be necessary to execute all of these actions to build the requested // targets. May not be populated if analysis phase was fully cached. int32 action_count = 2; + // How many actions belonged to configured targets: always at most + // action_count. Useful mainly for historical comparisons to + // ActionMetrics.actions_created, which used to not count aspects' actions. + int32 action_count_not_including_aspects = 6; // How many artifacts are outputs of the above actions. May not be populated // if analysis phase was fully cached. int32 output_artifact_count = 3;
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java index 6270462..0837b4f 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -51,7 +51,6 @@ import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.DetailedExitCode; -import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; import com.google.devtools.common.options.OptionsParsingException; import java.util.Collection; @@ -228,18 +227,15 @@ env.getReporter(), env.getEventBus()); - Pair<Integer, Integer> tcal = view.getTargetsConfiguredAndLoaded(); - // TODO(bazel-team): Merge these into one event. env.getEventBus() .post( new AnalysisPhaseCompleteEvent( analysisResult.getTargetsToBuild(), - /* targetsLoaded */ tcal.second.intValue(), - /* targetsConfigured */ tcal.first.intValue(), + view.getEvaluatedCounts(), + view.getEvaluatedActionsCounts(), timer.stop().elapsed(TimeUnit.MILLISECONDS), view.getAndClearPkgManagerStatistics(), - view.getActionsConstructed(), env.getSkyframeExecutor().wasAnalysisCacheDiscardedAndResetBit())); ImmutableSet<BuildConfigurationValue.Key> configurationKeys = Stream.concat(
diff --git a/src/main/java/com/google/devtools/build/lib/metrics/BUILD b/src/main/java/com/google/devtools/build/lib/metrics/BUILD index 8f3f65d..45af86c 100644 --- a/src/main/java/com/google/devtools/build/lib/metrics/BUILD +++ b/src/main/java/com/google/devtools/build/lib/metrics/BUILD
@@ -34,6 +34,7 @@ ":memory-use-recorder", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:analysis_graph_stats_event", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_complete_event", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_started_event", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
diff --git a/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java b/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java index 11db50d..06f634d 100644 --- a/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java +++ b/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java
@@ -17,6 +17,7 @@ import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.ActionCompletionEvent; import com.google.devtools.build.lib.actions.AnalysisGraphStatsEvent; +import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric; import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent; import com.google.devtools.build.lib.analysis.AnalysisPhaseStartedEvent; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics; @@ -84,10 +85,14 @@ @SuppressWarnings("unused") @Subscribe public void onAnalysisPhaseComplete(AnalysisPhaseCompleteEvent event) { - actionSummary.setActionsCreated(event.getActionsConstructed()); + TotalAndConfiguredTargetOnlyMetric actionsConstructed = event.getActionsConstructed(); + actionSummary + .setActionsCreated(actionsConstructed.total()) + .setActionsCreatedNotIncludingAspects(actionsConstructed.configuredTargetsOnly()); + TotalAndConfiguredTargetOnlyMetric targetsConfigured = event.getTargetsConfigured(); targetMetrics - .setTargetsLoaded(event.getTargetsLoaded()) - .setTargetsConfigured(event.getTargetsConfigured()); + .setTargetsConfigured(targetsConfigured.total()) + .setTargetsConfiguredNotIncludingAspects(targetsConfigured.configuredTargetsOnly()); packageMetrics.setPackagesLoaded(event.getPkgManagerStats().getPackagesLoaded()); timingMetrics.setAnalysisPhaseTimeInMs(event.getTimeInMs()); } @@ -95,9 +100,14 @@ @SuppressWarnings("unused") @Subscribe public synchronized void logAnalysisGraphStats(AnalysisGraphStatsEvent event) { + TotalAndConfiguredTargetOnlyMetric actionLookupValueCount = event.getActionLookupValueCount(); + TotalAndConfiguredTargetOnlyMetric actionCount = event.getActionCount(); buildGraphMetrics - .setActionLookupValueCount(event.getActionLookupValueCount()) - .setActionCount(event.getActionCount()) + .setActionLookupValueCount(actionLookupValueCount.total()) + .setActionLookupValueCountNotIncludingAspects( + actionLookupValueCount.configuredTargetsOnly()) + .setActionCount(actionCount.total()) + .setActionCountNotIncludingAspects(actionCount.configuredTargetsOnly()) .setOutputArtifactCount(event.getOutputArtifactCount()); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java index 801a2bc..15fb842 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactConflictFinder.java
@@ -29,6 +29,8 @@ import com.google.devtools.build.lib.actions.MapBasedActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric; +import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; import com.google.devtools.build.lib.concurrent.ExecutorUtil; import com.google.devtools.build.lib.concurrent.Sharder; import com.google.devtools.build.lib.concurrent.ThrowableRecordingRunnableWrapper; @@ -72,7 +74,7 @@ MapBasedActionGraph actionGraph = new MapBasedActionGraph(actionKeyContext); ConcurrentNavigableMap<PathFragment, Artifact> artifactPathMap = new ConcurrentSkipListMap<>(Actions.comparatorForPrefixConflicts()); - Pair<Integer, Integer> counts = + Pair<TotalAndConfiguredTargetOnlyMetric, TotalAndConfiguredTargetOnlyMetric> counts = constructActionGraphAndPathMap( actionGraph, artifactPathMap, actionLookupValues, temporaryBadActionMap); @@ -95,23 +97,30 @@ * PathFragment}s to their respective {@link Artifact}s. We do this in a threadpool to save around * 1.5 seconds on a mid-sized build versus a single-threaded operation. */ - private static Pair<Integer, Integer> constructActionGraphAndPathMap( - MutableActionGraph actionGraph, - ConcurrentNavigableMap<PathFragment, Artifact> artifactPathMap, - Iterable<ActionLookupValue> values, - ConcurrentMap<ActionAnalysisMetadata, ConflictException> badActionMap) - throws InterruptedException { + private static Pair<TotalAndConfiguredTargetOnlyMetric, TotalAndConfiguredTargetOnlyMetric> + constructActionGraphAndPathMap( + MutableActionGraph actionGraph, + ConcurrentNavigableMap<PathFragment, Artifact> artifactPathMap, + Iterable<ActionLookupValue> values, + ConcurrentMap<ActionAnalysisMetadata, ConflictException> badActionMap) + throws InterruptedException { // Action graph construction is CPU-bound. int numJobs = Runtime.getRuntime().availableProcessors(); // No great reason for expecting 5000 action lookup values, but not worth counting size of // values. Sharder<ActionLookupValue> actionShards = new Sharder<>(numJobs, 5000); int actionLookupValueCount = 0; + int configuredTargetValueCount = 0; int actionCount = 0; + int configuredTargetActionCount = 0; for (ActionLookupValue value : values) { actionShards.add(value); actionLookupValueCount++; actionCount += value.getNumActions(); + if (value instanceof ConfiguredTargetValue) { + configuredTargetValueCount++; + configuredTargetActionCount += value.getNumActions(); + } } ThrowableRecordingRunnableWrapper wrapper = @@ -131,7 +140,10 @@ if (interrupted) { throw new InterruptedException(); } - return Pair.of(actionLookupValueCount, actionCount); + return Pair.of( + TotalAndConfiguredTargetOnlyMetric.create( + actionLookupValueCount, configuredTargetValueCount), + TotalAndConfiguredTargetOnlyMetric.create(actionCount, configuredTargetActionCount)); } private static Runnable actionRegistration( @@ -206,8 +218,8 @@ private static ActionConflictsAndStats create( ImmutableMap<ActionAnalysisMetadata, ConflictException> conflicts, - int actionValueCount, - int actionCount, + TotalAndConfiguredTargetOnlyMetric actionValueCount, + TotalAndConfiguredTargetOnlyMetric actionCount, int artifactCount) { return new AutoValue_ArtifactConflictFinder_ActionConflictsAndStats( conflicts, new AnalysisGraphStatsEvent(actionValueCount, actionCount, artifactCount));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index be2e71c..3b42cd4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -765,7 +765,9 @@ deps = [ ":precomputed_value", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:analysis_graph_stats_event", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
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 6d9a29e..1161654 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
@@ -15,7 +15,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState.BUILT; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; @@ -37,6 +36,7 @@ import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.actions.PackageRoots; +import com.google.devtools.build.lib.actions.TotalAndConfiguredTargetOnlyMetric; import com.google.devtools.build.lib.analysis.AnalysisFailureEvent; import com.google.devtools.build.lib.analysis.AspectValue; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; @@ -123,17 +123,17 @@ private final ActionKeyContext actionKeyContext; private boolean enableAnalysis = false; - // This hack allows us to see when a configured target has been invalidated, and thus when the set - // of artifact conflicts needs to be recomputed (whenever a configured target has been invalidated - // or newly evaluated). - private final ConfiguredTargetValueProgressReceiver progressReceiver = - new ConfiguredTargetValueProgressReceiver(); + // This hack allows us to see when an action lookup node has been invalidated, and thus when the + // set of artifact conflicts needs to be recomputed (whenever an action lookup node has been + // invalidated or newly evaluated). + private final ActionLookupValueProgressReceiver progressReceiver = + new ActionLookupValueProgressReceiver(); // Used to see if checks of graph consistency need to be done after analysis. - private volatile boolean someConfiguredTargetEvaluated = false; + private volatile boolean someActionLookupValueEvaluated = false; - // We keep the set of invalidated configuration target keys so that we can know if something - // has been invalidated after graph pruning has been executed. - private Set<SkyKey> dirtiedConfiguredTargetKeys = Sets.newConcurrentHashSet(); + // We keep the set of invalidated action lookup nodes so that we can know if something has been + // invalidated after graph pruning has been executed. + private Set<ActionLookupKey> dirtiedActionLookupKeys = Sets.newConcurrentHashSet(); private final ConfiguredRuleClassProvider ruleClassProvider; @@ -171,16 +171,19 @@ progressReceiver.reset(); } - public ImmutableSet<SkyKey> getEvaluatedTargetKeys() { - return ImmutableSet.copyOf(progressReceiver.evaluatedConfiguredTargets); + public TotalAndConfiguredTargetOnlyMetric getEvaluatedCounts() { + return TotalAndConfiguredTargetOnlyMetric.create( + progressReceiver.actionLookupValueCount.get(), + progressReceiver.configuredTargetCount.get()); } ConfiguredTargetFactory getConfiguredTargetFactory() { return factory; } - public int getEvaluatedActionCount() { - return progressReceiver.evaluatedActionCount.get(); + public TotalAndConfiguredTargetOnlyMetric getEvaluatedActionCounts() { + return TotalAndConfiguredTargetOnlyMetric.create( + progressReceiver.actionCount.get(), progressReceiver.configuredTargetActionCount.get()); } /** @@ -450,7 +453,7 @@ actionKeyContext); eventBus.post(conflictsAndStats.getStats()); actionConflicts = conflictsAndStats.getConflicts(); - someConfiguredTargetEvaluated = false; + someActionLookupValueEvaluated = false; } } foundActionConflict = !actionConflicts.isEmpty(); @@ -556,13 +559,13 @@ } private boolean shouldCheckForConflicts(ImmutableSet<SkyKey> newKeys) { - if (someConfiguredTargetEvaluated) { + if (someActionLookupValueEvaluated) { // A top-level target was added and may introduce a conflict, or a top-level target was // recomputed and may introduce or resolve a conflict. return true; } - if (!dirtiedConfiguredTargetKeys.isEmpty()) { + if (!dirtiedActionLookupKeys.isEmpty()) { // No target was (re)computed but at least one was dirtied. // Example: (//:x //foo:y) are built, and in conflict (//:x creates foo/C and //foo:y // creates C). Then y is removed from foo/BUILD and only //:x is built, so //foo:y is @@ -1035,9 +1038,9 @@ return progressReceiver; } - /** Clear the invalidated configured targets detected during loading and analysis phases. */ - public void clearInvalidatedConfiguredTargets() { - dirtiedConfiguredTargetKeys = Sets.newConcurrentHashSet(); + /** Clear the invalidated action lookup nodes detected during loading and analysis phases. */ + public void clearInvalidatedActionLookupKeys() { + dirtiedActionLookupKeys = Sets.newConcurrentHashSet(); } /** @@ -1054,20 +1057,21 @@ return skyframeExecutor.getActionKeyContext(); } - private final class ConfiguredTargetValueProgressReceiver + private final class ActionLookupValueProgressReceiver extends EvaluationProgressReceiver.NullEvaluationProgressReceiver { - private final Set<SkyKey> evaluatedConfiguredTargets = Sets.newConcurrentHashSet(); - private final AtomicInteger evaluatedActionCount = new AtomicInteger(); + private final AtomicInteger actionLookupValueCount = new AtomicInteger(); + private final AtomicInteger actionCount = new AtomicInteger(); + private final AtomicInteger configuredTargetCount = new AtomicInteger(); + private final AtomicInteger configuredTargetActionCount = new AtomicInteger(); @Override public void invalidated(SkyKey skyKey, InvalidationState state) { - if (skyKey.functionName().equals(SkyFunctions.CONFIGURED_TARGET) - && state != InvalidationState.DELETED) { + if (skyKey instanceof ActionLookupKey && state != InvalidationState.DELETED) { // If the value was just dirtied and not deleted, then it may not be truly invalid, since // it may later get re-validated. Therefore adding the key to dirtiedConfiguredTargetKeys // is provisional--if the key is later evaluated and the value found to be clean, then we // remove it from the set. - dirtiedConfiguredTargetKeys.add(skyKey); + dirtiedActionLookupKeys.add((ActionLookupKey) skyKey); } } @@ -1077,34 +1081,42 @@ @Nullable SkyValue value, Supplier<EvaluationSuccessState> evaluationSuccessState, EvaluationState state) { - if (skyKey.functionName().equals(SkyFunctions.CONFIGURED_TARGET)) { - switch (state) { - case BUILT: - if (evaluationSuccessState.get().succeeded()) { - evaluatedConfiguredTargets.add(skyKey); - // During multithreaded operation, this is only set to true, so no concurrency issues. - someConfiguredTargetEvaluated = true; + // We tolerate any action lookup keys here, although we only expect configured targets, + // aspects, and the workspace status value. + if (!(skyKey instanceof ActionLookupKey)) { + return; + } + switch (state) { + case BUILT: + boolean isConfiguredTarget = skyKey.functionName().equals(SkyFunctions.CONFIGURED_TARGET); + if (evaluationSuccessState.get().succeeded()) { + actionLookupValueCount.incrementAndGet(); + if (isConfiguredTarget) { + configuredTargetCount.incrementAndGet(); } - if (value instanceof ConfiguredTargetValue) { - evaluatedActionCount.addAndGet(((ConfiguredTargetValue) value).getNumActions()); + // During multithreaded operation, this is only set to true, so no concurrency issues. + someActionLookupValueEvaluated = true; + } + if (value instanceof ActionLookupValue) { + int numActions = ((ActionLookupValue) value).getNumActions(); + actionCount.addAndGet(numActions); + if (isConfiguredTarget) { + configuredTargetActionCount.addAndGet(numActions); } - break; - case CLEAN: - // If the configured target value did not need to be rebuilt, then it wasn't truly - // invalid. - dirtiedConfiguredTargetKeys.remove(skyKey); - break; - } - } else if (skyKey.functionName().equals(SkyFunctions.ASPECT) - && state == BUILT - && value instanceof AspectValue) { - evaluatedActionCount.addAndGet(((AspectValue) value).getNumActions()); + } + break; + case CLEAN: + // If the action lookup value did not need to be rebuilt, then it wasn't truly invalid. + dirtiedActionLookupKeys.remove(skyKey); + break; } } public void reset() { - evaluatedConfiguredTargets.clear(); - evaluatedActionCount.set(0); + actionLookupValueCount.set(0); + actionCount.set(0); + configuredTargetCount.set(0); + configuredTargetActionCount.set(0); } } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 7f3d749..a275810 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -872,7 +872,7 @@ logger.atInfo().log("Dropping configured target data"); analysisCacheDiscarded = true; clearTrimmingCache(); - skyframeBuildView.clearInvalidatedConfiguredTargets(); + skyframeBuildView.clearInvalidatedActionLookupKeys(); skyframeBuildView.clearLegacyData(); ArtifactNestedSetFunction.getInstance().resetArtifactNestedSetFunctionMaps(); }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java index f1a4d12..d2a97a5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -920,4 +920,77 @@ setRulesAndAspectsAvailableInTests(ImmutableList.of(), ImmutableList.of()); getConfiguredTarget("//a:r"); } + + @Test + public void topLevelConflictDetected() throws Exception { + String bzlFileTemplate = + String.join( + "\n", + "def _aspect1_impl(target, ctx):", + " outfile = ctx.actions.declare_file('aspect.out')", + " ctx.actions.run_shell(", + " outputs = [outfile],", + " progress_message = 'Action for aspect 1',", + " command = 'echo \"1\" > ' + outfile.path,", + " )", + " return [OutputGroupInfo(files = [outfile])]", + "def _aspect2_impl(target, ctx):", + " outfile = ctx.actions.declare_file('aspect.out')", + " ctx.actions.run_shell(", + " outputs = [outfile],", + " progress_message = 'Action for aspect 2',", + " command = 'echo \"%s\" > ' + outfile.path,", + " )", + " return [OutputGroupInfo(files = [outfile])]", + "aspect1 = aspect(implementation = _aspect1_impl)", + "aspect2 = aspect(implementation = _aspect2_impl)"); + scratch.file("foo/aspect.bzl", String.format(bzlFileTemplate, "2")); + scratch.file("foo/BUILD", "sh_library(name = 'foo', srcs = ['foo.sh'])"); + // Expect errors. + reporter.removeHandler(failFastHandler); + ViewCreationFailedException exception = + assertThrows( + ViewCreationFailedException.class, + () -> + update( + new EventBus(), + defaultFlags(), + ImmutableList.of("//foo:aspect.bzl%aspect1", "//foo:aspect.bzl%aspect2"), + "//foo:foo")); + assertThat(exception) + .hasMessageThat() + .contains( + "ConflictException: for foo/aspect.out, previous action: action 'Action for aspect 1'," + + " attempted action: action 'Action for aspect 2'"); + + // Fix bzl file so actions are shared: analysis should succeed now. + scratch.overwriteFile("foo/aspect.bzl", String.format(bzlFileTemplate, "1")); + reporter.addHandler(failFastHandler); + AnalysisResult result = + update( + new EventBus(), + defaultFlags(), + ImmutableList.of("//foo:aspect.bzl%aspect1", "//foo:aspect.bzl%aspect2"), + "//foo:foo"); + assertThat(result.getAspectsMap()).hasSize(2); + + // Break bzl file again: we should notice. + scratch.overwriteFile("foo/aspect.bzl", String.format(bzlFileTemplate, "2")); + // Expect errors. + reporter.removeHandler(failFastHandler); + exception = + assertThrows( + ViewCreationFailedException.class, + () -> + update( + new EventBus(), + defaultFlags(), + ImmutableList.of("//foo:aspect.bzl%aspect1", "//foo:aspect.bzl%aspect2"), + "//foo:foo")); + assertThat(exception) + .hasMessageThat() + .contains( + "ConflictException: for foo/aspect.out, previous action: action 'Action for aspect 1'," + + " attempted action: action 'Action for aspect 2'"); + } }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD index 88c941b..555f696 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
@@ -38,6 +38,7 @@ deps = [ "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", + "//src/main/java/com/google/devtools/build/lib/actions:artifact_owner", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", @@ -69,7 +70,6 @@ "//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory", "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment", "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options", - "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_provider", "//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition", "//src/main/java/com/google/devtools/build/lib/analysis:config/per_label_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/run_under",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java index 95d72e7..f52a4ee 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java
@@ -22,7 +22,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultiset; +import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -45,7 +47,6 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; -import com.google.devtools.build.skyframe.SkyKey; import java.util.Arrays; import java.util.LinkedHashSet; import java.util.Map; @@ -138,11 +139,11 @@ } private void assertNumberOfConfigurationsOfTargets( - Set<SkyKey> keys, Map<String, Integer> targetsWithCounts) { + Set<ActionLookupKey> keys, Map<String, Integer> targetsWithCounts) { ImmutableMultiset<Label> actualSet = keys.stream() .filter(key -> key instanceof ConfiguredTargetKey) - .map(key -> ((ConfiguredTargetKey) key).getLabel()) + .map(ArtifactOwner::getLabel) .collect(toImmutableMultiset()); ImmutableMap<Label, Integer> expected = targetsWithCounts @@ -201,7 +202,8 @@ "//test:starlark_dep", "//test:native_shared_dep", "//test:starlark_shared_dep"); - LinkedHashSet<SkyKey> visitedTargets = new LinkedHashSet<>(getSkyframeEvaluatedTargetKeys()); + LinkedHashSet<ActionLookupKey> visitedTargets = + new LinkedHashSet<>(getSkyframeEvaluatedTargetKeys()); // asserting that the top-level targets are the same as the ones in the diamond starting at // //test:suite assertNumberOfConfigurationsOfTargets( @@ -335,7 +337,8 @@ "//test:starlark_dep", "//test:native_shared_dep", "//test:starlark_shared_dep"); - LinkedHashSet<SkyKey> visitedTargets = new LinkedHashSet<>(getSkyframeEvaluatedTargetKeys()); + LinkedHashSet<ActionLookupKey> visitedTargets = + new LinkedHashSet<>(getSkyframeEvaluatedTargetKeys()); // asserting that the top-level targets are the same as the ones in the diamond starting at // //test:suite assertNumberOfConfigurationsOfTargets( @@ -640,7 +643,8 @@ "//test:starlark_dep", "//test:native_shared_dep", "//test:starlark_shared_dep"); - LinkedHashSet<SkyKey> visitedTargets = new LinkedHashSet<>(getSkyframeEvaluatedTargetKeys()); + LinkedHashSet<ActionLookupKey> visitedTargets = + new LinkedHashSet<>(getSkyframeEvaluatedTargetKeys()); assertNumberOfConfigurationsOfTargets( visitedTargets, new ImmutableMap.Builder<String, Integer>()
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index ae61bb3..c0d04a8 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.actions.ActionLookupValue; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.analysis.AnalysisOptions; import com.google.devtools.build.lib.analysis.AnalysisResult; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -80,7 +81,6 @@ import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParser; import java.util.Arrays; @@ -568,8 +568,8 @@ .build()); } - protected Set<SkyKey> getSkyframeEvaluatedTargetKeys() { - return buildView.getSkyframeEvaluatedTargetKeysForTesting(); + protected Set<ActionLookupKey> getSkyframeEvaluatedTargetKeys() { + return buildView.getSkyframeEvaluatedActionLookupKeyCountForTesting(); } protected void assertNumberOfAnalyzedConfigurationsOfTargets( @@ -577,7 +577,7 @@ ImmutableMultiset<Label> actualSet = getSkyframeEvaluatedTargetKeys().stream() .filter(key -> key instanceof ConfiguredTargetKey) - .map(key -> ((ConfiguredTargetKey) key).getLabel()) + .map(ArtifactOwner::getLabel) .collect(toImmutableMultiset()); ImmutableMap<Label, Integer> expected = targetsWithCounts.entrySet().stream()
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java index 8d10729..4f51619 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -14,6 +14,9 @@ package com.google.devtools.build.lib.analysis.util; +import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.Streams.stream; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.Collections2; @@ -21,7 +24,11 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.MapDifference; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; +import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.PackageRoots; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; @@ -95,6 +102,7 @@ import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.ValueOrException; +import com.google.devtools.build.skyframe.Version; import java.util.Collection; import java.util.HashMap; import java.util.LinkedHashMap; @@ -119,6 +127,8 @@ private final ConfiguredRuleClassProvider ruleClassProvider; + private ImmutableMap<ActionLookupKey, Version> currentActionLookupKeys = ImmutableMap.of(); + public BuildViewForTesting( BlazeDirectories directories, ConfiguredRuleClassProvider ruleClassProvider, @@ -136,8 +146,28 @@ } @VisibleForTesting - public Set<SkyKey> getSkyframeEvaluatedTargetKeysForTesting() { - return skyframeBuildView.getEvaluatedTargetKeys(); + public Set<ActionLookupKey> getSkyframeEvaluatedActionLookupKeyCountForTesting() { + Set<ActionLookupKey> actionLookupKeys = populateActionLookupKeyMapAndGetDiff(); + Preconditions.checkState( + actionLookupKeys.size() == skyframeBuildView.getEvaluatedCounts().total(), + "Number of newly evaluated action lookup values %s does not agree with number that changed" + + " in graph: %s", + actionLookupKeys); + return actionLookupKeys; + } + + private Set<ActionLookupKey> populateActionLookupKeyMapAndGetDiff() { + ImmutableMap<ActionLookupKey, Version> newMap = + stream(skyframeExecutor.getEvaluatorForTesting().getGraphEntries()) + .filter(e -> e.getKey() instanceof ActionLookupKey) + .collect( + toImmutableMap( + e -> ((ActionLookupKey) e.getKey()), e -> e.getValue().getVersion())); + MapDifference<ActionLookupKey, Version> difference = + Maps.difference(newMap, currentActionLookupKeys); + currentActionLookupKeys = newMap; + return Sets.union( + difference.entriesDiffering().keySet(), difference.entriesOnlyOnLeft().keySet()); } /** @@ -162,6 +192,7 @@ ExtendedEventHandler eventHandler, EventBus eventBus) throws ViewCreationFailedException, InterruptedException, InvalidConfigurationException { + populateActionLookupKeyMapAndGetDiff(); return buildView.update( loadingResult, targetOptions,