Show configured targets summary during analysis phase. Developers like to know what's going on when they are building their targets. Currently, especially when building complex targets, there is no update shown on the command line during the analysis phase when all packages are loaded and targets are still being configured. Add the number of configured targets after the number of loaded packages to show the developer what's happening during that time. RELNOTES: Add number of configured targets to analysis phase status output. PiperOrigin-RevId: 213786479
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java index d19eb91..3bb8a48 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress; import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent; +import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent; import com.google.devtools.build.lib.skyframe.LoadingPhaseStartedEvent; import com.google.devtools.build.lib.util.io.AnsiTerminal; import com.google.devtools.build.lib.util.io.AnsiTerminal.Color; @@ -597,6 +598,16 @@ } @Subscribe + public void configurationStarted(ConfigurationPhaseStartedEvent event) { + maybeAddDate(); + stateTracker.configurationStarted(event); + // As a new phase started, inform immediately. + ignoreRefreshLimitOnce(); + refresh(); + startUpdateThread(); + } + + @Subscribe public void loadingComplete(LoadingPhaseCompleteEvent event) { stateTracker.loadingComplete(event); refresh();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java index 79bb3d8..7aa35cf 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java
@@ -36,6 +36,8 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress; import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent; +import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetProgressReceiver; import com.google.devtools.build.lib.skyframe.LoadingPhaseStartedEvent; import com.google.devtools.build.lib.skyframe.PackageProgressReceiver; import com.google.devtools.build.lib.util.Pair; @@ -119,6 +121,7 @@ private ExecutionProgressReceiver executionProgressReceiver; private PackageProgressReceiver packageProgressReceiver; + private ConfiguredTargetProgressReceiver configuredTargetProgressReceiver; // Set of build event protocol transports that need yet to be closed. private Set<BuildEventTransport> bepOpenTransports = new HashSet<>(); @@ -164,6 +167,10 @@ packageProgressReceiver = event.getPackageProgressReceiver(); } + void configurationStarted(ConfigurationPhaseStartedEvent event) { + configuredTargetProgressReceiver = event.getConfiguredTargetProgressReceiver(); + } + void loadingComplete(LoadingPhaseCompleteEvent event) { int count = event.getLabels().size(); status = "Analyzing"; @@ -182,11 +189,16 @@ String workDone = "Analysed " + additionalMessage; if (packageProgressReceiver != null) { Pair<String, String> progress = packageProgressReceiver.progressState(); - workDone += " (" + progress.getFirst() + ")"; + workDone += " (" + progress.getFirst(); + if (configuredTargetProgressReceiver != null) { + workDone += ", " + configuredTargetProgressReceiver.getProgressString(); + } + workDone += ")"; } workDone += "."; status = null; packageProgressReceiver = null; + configuredTargetProgressReceiver = null; return workDone; } @@ -805,7 +817,11 @@ terminalWriter.append(status + ":").normal().append(" " + additionalMessage); if (packageProgressReceiver != null) { Pair<String, String> progress = packageProgressReceiver.progressState(); - terminalWriter.append(" (" + progress.getFirst() + ")"); + terminalWriter.append(" (" + progress.getFirst()); + if (configuredTargetProgressReceiver != null) { + terminalWriter.append(", " + configuredTargetProgressReceiver.getProgressString()); + } + terminalWriter.append(")"); if (progress.getSecond().length() > 0 && !shortVersion) { terminalWriter.newline().append(" " + progress.getSecond()); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationPhaseStartedEvent.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationPhaseStartedEvent.java new file mode 100644 index 0000000..96673be --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationPhaseStartedEvent.java
@@ -0,0 +1,36 @@ +// Copyright 2018 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.skyframe; + +import com.google.devtools.build.lib.events.ExtendedEventHandler; + +/** This event is fired at the beginning of the target configuration phase. */ +public final class ConfigurationPhaseStartedEvent implements ExtendedEventHandler.Postable { + + final ConfiguredTargetProgressReceiver configuredTargetProgress; + + /** + * Construct the event. + * + * @param configuredTargetProgress a receiver that gets updated about the progress of target + * configuration. + */ + public ConfigurationPhaseStartedEvent(ConfiguredTargetProgressReceiver configuredTargetProgress) { + this.configuredTargetProgress = configuredTargetProgress; + } + + public ConfiguredTargetProgressReceiver getConfiguredTargetProgressReceiver() { + return configuredTargetProgress; + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index f88f7b6..87763a2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -129,6 +129,7 @@ private final RuleClassProvider ruleClassProvider; private final Semaphore cpuBoundSemaphore; private final BuildOptions defaultBuildOptions; + @Nullable private final ConfiguredTargetProgressReceiver configuredTargetProgress; /** * Indicates whether the set of packages transitively loaded for a given {@link * ConfiguredTargetValue} will be needed for package root resolution later in the build. If not, @@ -144,7 +145,8 @@ Semaphore cpuBoundSemaphore, boolean storeTransitivePackagesForPackageRootResolution, boolean shouldUnblockCpuWorkWhenFetchingDeps, - BuildOptions defaultBuildOptions) { + BuildOptions defaultBuildOptions, + @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; this.cpuBoundSemaphore = cpuBoundSemaphore; @@ -152,6 +154,7 @@ storeTransitivePackagesForPackageRootResolution; this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps; this.defaultBuildOptions = defaultBuildOptions; + this.configuredTargetProgress = configuredTargetProgress; } @Override @@ -329,6 +332,9 @@ configConditions, toolchainContext, transitivePackagesForPackageRootResolution); + if (configuredTargetProgress != null) { + configuredTargetProgress.doneConfigureTarget(); + } return ans; } catch (DependencyEvaluationException e) { if (e.getCause() instanceof ConfiguredValueCreationException) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetProgressReceiver.java new file mode 100644 index 0000000..1be6275 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetProgressReceiver.java
@@ -0,0 +1,49 @@ +// Copyright 2018 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.skyframe; + +import java.util.concurrent.atomic.AtomicInteger; + +/** + * A class that, when being told the end of a target being configured, keeps track of the + * configuration progress and provides it as a human-readable string intended for the progress bar. + */ +public class ConfiguredTargetProgressReceiver { + + private AtomicInteger configuredTargetsCompleted = new AtomicInteger(); + + /** Register that a target has been configured. */ + void doneConfigureTarget() { + configuredTargetsCompleted.incrementAndGet(); + } + + /** + * Reset all instance variables of this object to a state equal to that of a newly + * constructed object. + */ + public void reset() { + configuredTargetsCompleted.set(0); + } + + /** + * Return a snapshot of the configuration progress as human-readable description of the number of + * targets configured so far. + */ + public String getProgressString() { + String progress = "" + configuredTargetsCompleted + " "; + progress += (configuredTargetsCompleted.get() != 1) ? "targets" : "target"; + progress += " configured"; + return progress; + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 19e6311..182ea22 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -171,7 +171,8 @@ GraphInconsistencyReceiver.THROWING, defaultBuildOptions, new PackageProgressReceiver(), - mutableArtifactFactorySupplier); + mutableArtifactFactorySupplier, + new ConfiguredTargetProgressReceiver()); this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories); this.customDirtinessCheckers = customDirtinessCheckers; }
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 85d8739..1fc58fb 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
@@ -230,6 +230,7 @@ private final AtomicInteger numPackagesLoaded = new AtomicInteger(0); @Nullable private final PackageProgressReceiver packageProgress; + @Nullable private final ConfiguredTargetProgressReceiver configuredTargetProgress; private final SkyframeBuildView skyframeBuildView; private ActionLogBufferPathGenerator actionLogBufferPathGenerator; @@ -363,7 +364,8 @@ GraphInconsistencyReceiver graphInconsistencyReceiver, BuildOptions defaultBuildOptions, @Nullable PackageProgressReceiver packageProgress, - MutableArtifactFactorySupplier artifactResolverSupplier) { + MutableArtifactFactorySupplier artifactResolverSupplier, + @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress) { // Strictly speaking, these arguments are not required for initialization, but all current // callsites have them at hand, so we might as well set them during construction. this.evaluatorSupplier = evaluatorSupplier; @@ -406,6 +408,7 @@ this.buildFilesByPriority = buildFilesByPriority; this.actionOnIOExceptionReadingBuildFile = actionOnIOExceptionReadingBuildFile; this.packageProgress = packageProgress; + this.configuredTargetProgress = configuredTargetProgress; } private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions( @@ -489,7 +492,8 @@ cpuBoundSemaphore, shouldStoreTransitivePackagesInLoadingAndAnalysis(), shouldUnblockCpuWorkWhenFetchingDeps, - defaultBuildOptions)); + defaultBuildOptions, + configuredTargetProgress)); map.put( SkyFunctions.ASPECT, new AspectFunction( @@ -1882,6 +1886,7 @@ for (AspectValueKey aspectKey : aspectKeys) { keys.add(aspectKey); } + eventHandler.post(new ConfigurationPhaseStartedEvent(configuredTargetProgress)); EvaluationResult<ActionLookupValue> result = buildDriver.evaluate( keys, @@ -2355,6 +2360,10 @@ "Unknown error during configuration creation evaluation", e); } + if (configuredTargetProgress != null) { + configuredTargetProgress.reset(); + } + PrepareAnalysisPhaseValue prepareAnalysisPhaseValue = evalResult.get(key); return prepareAnalysisPhaseValue; }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetProgressReceiverTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetProgressReceiverTest.java new file mode 100644 index 0000000..eeaae02 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetProgressReceiverTest.java
@@ -0,0 +1,56 @@ +// Copyright 2018 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.skyframe; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests {@link ConfiguredTargetProgressReceiver}. */ +@RunWith(JUnit4.class) +public class ConfiguredTargetProgressReceiverTest { + + @Test + public void testTargetCounted() { + // If the configuration of a target is completed it is counted as fully configured target. + ConfiguredTargetProgressReceiver progress = new ConfiguredTargetProgressReceiver(); + progress.doneConfigureTarget(); + String progressString1 = progress.getProgressString(); + + assertWithMessage("One configured target should be visible in progress.") + .that(progressString1.equals("1 target configured")) + .isTrue(); + + progress.doneConfigureTarget(); + String progressString2 = progress.getProgressString(); + + assertWithMessage("Two configured targets should be visible in progress.") + .that(progressString2.equals("2 targets configured")) + .isTrue(); + } + + @Test + public void testReset() { + // After resetting, messages should be as immediately after creation. + ConfiguredTargetProgressReceiver progress = new ConfiguredTargetProgressReceiver(); + String defaultProgress = progress.getProgressString(); + progress.doneConfigureTarget(); + assertThat(progress.getProgressString()).isNotEqualTo(defaultProgress); + progress.reset(); + assertThat(progress.getProgressString()).isEqualTo(defaultProgress); + } +}