Skip to content

Commit

Permalink
Remove debugging for b/128541100, since the cause is now understood a…
Browse files Browse the repository at this point in the history
…nd benign (nonexistence of a node is cached in a SkyFunctionEnvironment for the duration of the SkyFunction evaluation).

PiperOrigin-RevId: 243353422
  • Loading branch information
janakdr authored and copybara-github committed Apr 12, 2019
1 parent f7c1e85 commit 9c0a633
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.DuplicateException;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition.TransitionException;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.buildeventstream.BuildEventId;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId;
import com.google.devtools.build.lib.causes.AnalysisFailedCause;
Expand Down Expand Up @@ -215,9 +214,6 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
return null;
}
PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.get(packageKey);
if (label.equals(labelWithUndonePackageToDiagnoseBug)) {
logger.atInfo().log("Retrieved values %s for %s", packageAndMaybeConfigurationValues, key);
}
if (configurationKeyMaybe != null) {
configuration =
((BuildConfigurationValue) packageAndMaybeConfigurationValues.get(configurationKeyMaybe))
Expand Down Expand Up @@ -717,27 +713,12 @@ private static Map<SkyKey, ConfiguredTargetAndData> resolveConfiguredTargetDepen
} else {
pkgValue = (PackageValue) packageResult.get();
if (pkgValue == null) {
logger.atInfo().log("Missing package: %s for (%s %s)", packageKey, dep, key);
// In a race, the getValuesOrThrow call above may have retrieved the package
// before it was done but the configured target after it was done. However, the
// configured target being done implies that the package is now done, so we can
// retrieve it from the graph.
pkgValue = (PackageValue) env.getValue(packageKey);
if (pkgValue == null) {
BugReport.sendBugReport(
new IllegalStateException(
"Package should have been loaded during dep resolution: "
+ dep
+ ", ("
+ depValue
+ ", "
+ packageResult
+ ", "
+ ctgValue
+ ")"));
missedValues = true;
continue;
}
// before it was done but the configured target after it was done. Since
// SkyFunctionEnvironment may cache absent values, re-requesting it on this
// evaluation may be useless, just treat it as missing.
missedValues = true;
continue;
}
}
} else {
Expand Down Expand Up @@ -952,7 +933,4 @@ private ConfiguredTargetFunctionException(ActionConflictException e) {
super(e, Transience.PERSISTENT);
}
}

// TODO(b/128541100): remove when bug is fixed.
public static Label labelWithUndonePackageToDiagnoseBug = null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,6 @@ private DirtyOutcome maybeHandleDirtyNode(NodeEntry state) throws InterruptedExc
// No child has a changed value. This node can be marked done and its parents signaled
// without any re-evaluation.
Set<SkyKey> reverseDeps = state.markClean();
if (matchesMissingSkyKey(skyKey)) {
logger.info(
"Marked " + skyKey + " clean: " + state + ", " + System.identityHashCode(state));
}
// Tell the receiver that the value was not actually changed this run.
evaluatorContext
.getProgressReceiver()
Expand Down Expand Up @@ -399,15 +395,6 @@ public void run() {
NodeEntry state =
Preconditions.checkNotNull(graph.get(null, Reason.EVALUATION, skyKey), skyKey);
Preconditions.checkState(state.isReady(), "%s %s", skyKey, state);
if (matchesMissingSkyKey(skyKey)) {
logger.info(
"Starting to evaluate "
+ skyKey
+ " with "
+ state
+ ", "
+ System.identityHashCode(state));
}
try {
evaluatorContext.getProgressReceiver().stateStarting(skyKey, NodeState.CHECK_DIRTY);
if (maybeHandleDirtyNode(state) == DirtyOutcome.ALREADY_PROCESSED) {
Expand Down Expand Up @@ -1035,11 +1022,4 @@ static BigInteger composeDepFingerprints(
static boolean isDoneForBuild(@Nullable NodeEntry entry) {
return entry != null && entry.isDone();
}

// TODO(b/128541100): Clean this up when bug is fixed.
public static SkyKey missingSkyKeyToDiagnoseBug = null;

static boolean matchesMissingSkyKey(SkyKey key) {
return (missingSkyKeyToDiagnoseBug != null && missingSkyKeyToDiagnoseBug.equals(key));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -785,11 +785,6 @@ Set<SkyKey> commit(NodeEntry primaryEntry, EnqueueParentBehavior enqueueParents)
// the data being written now is the same as the data already present in the entry.
Set<SkyKey> reverseDeps =
primaryEntry.setValue(valueWithMetadata, evaluationVersion, depFingerprintList);
if (AbstractParallelEvaluator.matchesMissingSkyKey(skyKey)) {
logger.atInfo().log(
"Set value for %s with %s (%s)",
skyKey, primaryEntry, System.identityHashCode(primaryEntry));
}

// Note that if this update didn't actually change the entry, this version may not be
// evaluationVersion.
Expand Down

0 comments on commit 9c0a633

Please sign in to comment.