diff options
Diffstat (limited to 'src/main')
8 files changed, 157 insertions, 43 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java b/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java index 35c642455e..8c383b2292 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/AbstractCriticalPathComponent.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.runtime; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionOwner; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.util.Clock; import com.google.devtools.build.lib.util.Preconditions; @@ -29,13 +31,13 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone // These two fields are values of BlazeClock.nanoTime() at the relevant points in time. private long startNanos; private long finishNanos = 0; - protected volatile boolean isRunning = true; + volatile boolean isRunning = true; /** We keep here the critical path time for the most expensive child. */ private long childAggregatedElapsedTime = 0; - /** The action for which we are storing the stat. */ - private final Action action; + /** May be nulled out after finished running to allow the action to be GC'ed. */ + @Nullable protected Action action; /** * Child with the maximum critical path. @@ -68,11 +70,35 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone return false; } - /** The action for which we are storing the stat. */ - public Action getAction() { + /** + * The action for which we are storing the stat. May be null if the action has finished running. + */ + @Nullable + public final Action maybeGetAction() { return action; } + public String prettyPrintAction() { + return getActionNotNull().prettyPrint(); + } + + @Nullable + public Label getOwner() { + ActionOwner owner = getActionNotNull().getOwner(); + if (owner != null && owner.getLabel() != null) { + return owner.getLabel(); + } + return null; + } + + public String getMnemonic() { + return getActionNotNull().getMnemonic(); + } + + private Action getActionNotNull() { + return Preconditions.checkNotNull(action, this); + } + /** * Add statistics for one dependency of this action. Caller should ensure {@code dep} not * running. @@ -95,7 +121,7 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone } long getElapsedTimeNanos() { - Preconditions.checkState(!isRunning, "Still running %s", action); + Preconditions.checkState(!isRunning, "Still running %s", this); return finishNanos - startNanos; } @@ -109,7 +135,7 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone } long getAggregatedElapsedTimeNanos() { - Preconditions.checkState(!isRunning, "Still running %s", action); + Preconditions.checkState(!isRunning, "Still running %s", this); return getElapsedTimeNanos() + childAggregatedElapsedTime; } @@ -132,7 +158,7 @@ public class AbstractCriticalPathComponent<C extends AbstractCriticalPathCompone if (!isRunning) { currentTime = String.format("%.2f", getElapsedTimeMillis() / 1000.0) + "s "; } - return currentTime + action.describe(); + return currentTime + prettyPrintAction(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ActionDiscardingCriticalPathComponent.java b/src/main/java/com/google/devtools/build/lib/runtime/ActionDiscardingCriticalPathComponent.java new file mode 100644 index 0000000000..200846fa68 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/ActionDiscardingCriticalPathComponent.java @@ -0,0 +1,64 @@ +// Copyright 2017 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.runtime; + +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.cmdline.Label; +import javax.annotation.Nullable; + +/** + * {@link SimpleCriticalPathComponent} that clears its {@link Action} reference after the component + * is finished running. This allows the action to be GC'ed, if other Bazel components are configured + * to also release their references to the action. + * + * <p>This class is separate from {@link SimpleCriticalPathComponent} for memory reasons: the + * additional references here add between 8 and 12 bytes per component instance with compressed OOPS + * and 24 bytes without compressed OOPS, which is a price we'd rather not pay unless the user + * explicitly enables this clearing of {@link Action} objects. + */ +class ActionDiscardingCriticalPathComponent extends SimpleCriticalPathComponent { + @Nullable private final Label owner; + private final String prettyPrint; + private final String mnemonic; + + ActionDiscardingCriticalPathComponent(Action action, long relativeStartNanos) { + super(action, relativeStartNanos); + this.prettyPrint = super.prettyPrintAction(); + this.owner = super.getOwner(); + this.mnemonic = super.getMnemonic(); + } + + @Override + public String getMnemonic() { + return mnemonic; + } + + @Override + public String prettyPrintAction() { + return prettyPrint; + } + + @Nullable + @Override + public Label getOwner() { + return owner; + } + + @Override + public synchronized boolean finishActionExecution(long startNanos, long finishNanos) { + boolean result = super.finishActionExecution(startNanos, finishNanos); + this.action = null; + return result; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java index 705a00ab15..0061e3f937 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java @@ -41,6 +41,7 @@ public class BuildSummaryStatsModule extends BlazeModule { private EventBus eventBus; private Reporter reporter; private boolean enabled; + private boolean discardActions; @Override public void beforeCommand(Command command, CommandEnvironment env) { @@ -59,12 +60,13 @@ public class BuildSummaryStatsModule extends BlazeModule { @Override public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) { enabled = env.getOptions().getOptions(ExecutionOptions.class).enableCriticalPathProfiling; + discardActions = !env.getSkyframeExecutor().hasIncrementalState(); } @Subscribe public void executionPhaseStarting(ExecutionStartingEvent event) { if (enabled) { - criticalPathComputer = new SimpleCriticalPathComputer(BlazeClock.instance()); + criticalPathComputer = new SimpleCriticalPathComputer(BlazeClock.instance(), discardActions); eventBus.register(criticalPathComputer); } } @@ -88,10 +90,12 @@ public class BuildSummaryStatsModule extends BlazeModule { // when the actions were executed while critical path computation is stored in the reverse // way. for (SimpleCriticalPathComponent stat : criticalPath.components().reverse()) { - Profiler.instance().logSimpleTaskDuration( - stat.getStartNanos(), - stat.getElapsedTimeNanos(), - ProfilerTask.CRITICAL_PATH_COMPONENT, stat.getAction()); + Profiler.instance() + .logSimpleTaskDuration( + stat.getStartNanos(), + stat.getElapsedTimeNanos(), + ProfilerTask.CRITICAL_PATH_COMPONENT, + stat.prettyPrintAction()); } Profiler.instance().completeTask(ProfilerTask.CRITICAL_PATH); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 46c133fcc8..cb2c80416b 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -32,7 +32,6 @@ import com.google.devtools.build.lib.analysis.config.DefaultsPackage; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Reporter; -import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.OutputService; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.Target; @@ -572,8 +571,7 @@ public final class CommandEnvironment { // Let skyframe figure out if it needs to store graph edges for this build. skyframeExecutor.decideKeepIncrementalState( runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class).batch, - optionsParser.getOptions(BuildView.Options.class), - optionsParser.getOptions(ExecutionOptions.class)); + optionsParser.getOptions(BuildView.Options.class)); // Start the performance and memory profilers. runtime.beforeCommand(this, options, execStartTimeNanos); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java index ad6d27ed83..0702fd198f 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CriticalPathComputer.java @@ -51,6 +51,7 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone /** Maximum critical path found. */ private C maxCriticalPath; private final Clock clock; + protected final boolean discardActions; /** * The list of slowest individual components, ignoring the time to build dependencies. @@ -69,8 +70,9 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone private final Object lock = new Object(); - protected CriticalPathComputer(Clock clock) { + protected CriticalPathComputer(Clock clock, boolean discardActions) { this.clock = clock; + this.discardActions = discardActions; maxCriticalPath = null; } @@ -122,17 +124,43 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone * @return The component to be used for updating the time stats. */ private C tryAddComponent(C newComponent) { - Action newAction = newComponent.getAction(); + Action newAction = Preconditions.checkNotNull(newComponent.maybeGetAction(), newComponent); Artifact primaryOutput = newAction.getPrimaryOutput(); C storedComponent = outputArtifactToComponent.putIfAbsent(primaryOutput, newComponent); if (storedComponent != null) { - if (!Actions.canBeShared(newAction, storedComponent.getAction())) { - throw new IllegalStateException("Duplicate output artifact found for unsharable actions." - + "This could happen if a previous event registered the action.\n" - + "Old action: " + storedComponent.getAction() + "\n\n" - + "New action: " + newAction + "\n\n" - + "Artifact: " + primaryOutput + "\n"); + Action oldAction = storedComponent.maybeGetAction(); + if (oldAction != null) { + if (!Actions.canBeShared(newAction, oldAction)) { + throw new IllegalStateException( + "Duplicate output artifact found for unsharable actions." + + "This can happen if a previous event registered the action.\n" + + "Old action: " + + oldAction + + "\n\nNew action: " + + newAction + + "\n\nArtifact: " + + primaryOutput + + "\n"); + } + } else { + String mnemonic = storedComponent.getMnemonic(); + String prettyPrint = storedComponent.prettyPrintAction(); + if (!newAction.getMnemonic().equals(mnemonic) + || !newAction.prettyPrint().equals(prettyPrint)) { + throw new IllegalStateException( + "Duplicate output artifact found for unsharable actions." + + "This can happen if a previous event registered the action.\n" + + "Old action mnemonic and prettyPrint: " + + mnemonic + + ", " + + prettyPrint + + "\n\nNew action: " + + newAction + + "\n\nArtifact: " + + primaryOutput + + "\n"); + } } } else { storedComponent = newComponent; @@ -197,12 +225,12 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone } private void finalizeActionStat(long startTimeNanos, Action action, C component) { - boolean updated = component.finishActionExecution(startTimeNanos, clock.nanoTime()); for (Artifact input : action.getInputs()) { addArtifactDependency(component, input); } + boolean updated = component.finishActionExecution(startTimeNanos, clock.nanoTime()); synchronized (lock) { if (isBiggestCriticalPath(component)) { maxCriticalPath = component; @@ -243,10 +271,10 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone private void addArtifactDependency(C actionStats, Artifact input) { C depComponent = outputArtifactToComponent.get(input); if (depComponent != null) { - if (depComponent.isRunning) { + Action action = depComponent.maybeGetAction(); + if (depComponent.isRunning && action != null) { // Rare case that an action depending on a previously-cached shared action sees a different // shared action that is in the midst of being an action cache hit. - Action action = depComponent.getAction(); for (Artifact actionOutput : action.getOutputs()) { if (input.equals(actionOutput) && Objects.equals(input.getArtifactOwner(), actionOutput.getArtifactOwner())) { @@ -255,7 +283,7 @@ public abstract class CriticalPathComputer<C extends AbstractCriticalPathCompone throw new IllegalStateException( String.format( "Cannot add critical path stats when the action is not finished. %s. %s. %s", - input, actionStats.getAction(), depComponent.getAction())); + input, actionStats.prettyPrintAction(), action)); } } return; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java b/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java index 9ceefa979b..df80bc143d 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/SimpleCriticalPathComputer.java @@ -24,13 +24,15 @@ public class SimpleCriticalPathComputer extends CriticalPathComputer<SimpleCriticalPathComponent, AggregatedCriticalPath<SimpleCriticalPathComponent>> { - public SimpleCriticalPathComputer(Clock clock) { - super(clock); + SimpleCriticalPathComputer(Clock clock, boolean discardActions) { + super(clock, discardActions); } @Override public SimpleCriticalPathComponent createComponent(Action action, long relativeStartNanos) { - return new SimpleCriticalPathComponent(action, relativeStartNanos); + return discardActions + ? new ActionDiscardingCriticalPathComponent(action, relativeStartNanos) + : new SimpleCriticalPathComponent(action, relativeStartNanos); } /** 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 aed5573cdf..e6b18384e7 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 @@ -26,7 +26,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.BlazeDirectories; -import com.google.devtools.build.lib.analysis.BuildView; +import com.google.devtools.build.lib.analysis.BuildView.Options; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory; @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.analysis.config.BinTools; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.concurrent.Uninterruptibles; import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Preprocessor; @@ -97,7 +96,6 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { private enum IncrementalState { NORMAL, - CLEAR_EDGES, CLEAR_EDGES_AND_ACTIONS } @@ -550,8 +548,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { } @Override - public void decideKeepIncrementalState( - boolean batch, BuildView.Options viewOptions, ExecutionOptions executionOptions) { + public void decideKeepIncrementalState(boolean batch, Options viewOptions) { Preconditions.checkState(!active); if (viewOptions == null) { // Some blaze commands don't include the view options. Don't bother with them. @@ -562,10 +559,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { incrementalState == IncrementalState.NORMAL, "May only be called once if successful: %s", incrementalState); - incrementalState = - executionOptions.enableCriticalPathProfiling - ? IncrementalState.CLEAR_EDGES - : IncrementalState.CLEAR_EDGES_AND_ACTIONS; + incrementalState = IncrementalState.CLEAR_EDGES_AND_ACTIONS; // Graph will be recreated on next sync. LOG.info("Set incremental state to " + incrementalState); } 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 fa7fccc86c..b5bda26ca8 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 @@ -80,7 +80,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.Reporter; -import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.OutputService; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.Attribute; @@ -659,8 +658,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { * way). * </ol> */ - public void decideKeepIncrementalState( - boolean batch, Options viewOptions, ExecutionOptions executionOptions) { + public void decideKeepIncrementalState(boolean batch, Options viewOptions) { // Assume incrementality. } |