Skip to content

Commit

Permalink
Remove dead DirtyResult#getOldValue and use singletons for "dirty wit…
Browse files Browse the repository at this point in the history
…hout new value" and "not dirty". #getOldValue has been dead since 15c55d5.

PiperOrigin-RevId: 433586219
  • Loading branch information
janakdr authored and copybara-github committed Mar 9, 2022
1 parent 5576979 commit 5a4b3dd
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,17 @@ public DirtyResult check(
RepositoryDirectoryValue repositoryValue = (RepositoryDirectoryValue) skyValue;

if (!repositoryValue.repositoryExists()) {
return DirtyResult.notDirty(skyValue);
return DirtyResult.notDirty();
}
if (repositoryValue.isFetchingDelayed()) {
return DirtyResult.dirty(skyValue);
return DirtyResult.dirty();
}

if (!managedDirectoriesExist(
workspaceRoot, managedDirectoriesKnowledge.getManagedDirectories(repositoryName))) {
return DirtyResult.dirty(skyValue);
return DirtyResult.dirty();
}
return DirtyResult.notDirty(skyValue);
return DirtyResult.notDirty();
}

static boolean managedDirectoriesExist(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,17 +148,17 @@ public SkyValueDirtinessChecker.DirtyResult check(
@Nullable TimestampGranularityMonitor tsgm) {
SkyValue newValue = super.createNewValue(skyKey, syscallCache, tsgm);
if (Objects.equal(newValue, oldValue)) {
return SkyValueDirtinessChecker.DirtyResult.notDirty(oldValue);
return SkyValueDirtinessChecker.DirtyResult.notDirty();
}
FileType fileType = externalFilesHelper.getAndNoteFileType((RootedPath) skyKey.argument());
if (fileType == FileType.EXTERNAL_REPO
|| fileType == FileType.EXTERNAL_IN_MANAGED_DIRECTORY) {
// Files under output_base/external have a dependency on the WORKSPACE file, so we don't add
// a new SkyValue to the graph yet because it might change once the WORKSPACE file has been
// parsed.
return SkyValueDirtinessChecker.DirtyResult.dirty(oldValue);
return SkyValueDirtinessChecker.DirtyResult.dirty();
}
return SkyValueDirtinessChecker.DirtyResult.dirtyWithNewValue(oldValue, newValue);
return SkyValueDirtinessChecker.DirtyResult.dirtyWithNewValue(newValue);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;

import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Objects;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -49,59 +51,55 @@ public DirtyResult check(
@Nullable TimestampGranularityMonitor tsgm) {
SkyValue newValue = createNewValue(key, syscallCache, tsgm);
if (newValue == null) {
return DirtyResult.dirty(oldValue);
return DirtyResult.dirty();
}
return newValue.equals(oldValue)
? DirtyResult.notDirty(oldValue)
: DirtyResult.dirtyWithNewValue(oldValue, newValue);
? DirtyResult.notDirty()
: DirtyResult.dirtyWithNewValue(newValue);
}

/** An encapsulation of the result of checking to see if a value is up to date. */
public static class DirtyResult {
public static final class DirtyResult {
private static final DirtyResult NOT_DIRTY =
new DirtyResult(/*isDirty=*/ false, /*newValue=*/ null);
private static final DirtyResult DIRTY = new DirtyResult(/*isDirty=*/ true, /*newValue=*/ null);

/**
* Creates a DirtyResult indicating that the external value is the same as the value in the
* graph.
*/
public static DirtyResult notDirty(SkyValue oldValue) {
return new DirtyResult(/*isDirty=*/false, oldValue, /*newValue=*/null);
public static DirtyResult notDirty() {
return NOT_DIRTY;
}

/**
* Creates a DirtyResult indicating that external value is different from the value in the
* graph, but this new value is not known.
*/
public static DirtyResult dirty(@Nullable SkyValue oldValue) {
return new DirtyResult(/*isDirty=*/true, oldValue, /*newValue=*/null);
public static DirtyResult dirty() {
return DIRTY;
}

/**
* Creates a DirtyResult indicating that the external value is {@code newValue}, which is
* different from the value in the graph,
*/
public static DirtyResult dirtyWithNewValue(@Nullable SkyValue oldValue, SkyValue newValue) {
return new DirtyResult(/*isDirty=*/true, oldValue, newValue);
public static DirtyResult dirtyWithNewValue(SkyValue newValue) {
return new DirtyResult(/*isDirty=*/ true, newValue);
}

private final boolean isDirty;
@Nullable private final SkyValue oldValue;
@Nullable private final SkyValue newValue;

private DirtyResult(boolean isDirty, @Nullable SkyValue oldValue,
@Nullable SkyValue newValue) {
private DirtyResult(boolean isDirty, @Nullable SkyValue newValue) {
this.isDirty = isDirty;
this.oldValue = oldValue;
this.newValue = newValue;
}

public boolean isDirty() {
return isDirty;
}

@Nullable
SkyValue getOldValue() {
return oldValue;
}

/**
* If {@code isDirty()}, then either returns the new value for the value or {@code null} if
* the new value wasn't computed. In the case where the value is dirty and a new value is
Expand All @@ -113,5 +111,30 @@ SkyValue getNewValue() {
Preconditions.checkState(isDirty(), newValue);
return newValue;
}

@Override
public int hashCode() {
return Objects.hashCode(newValue) + (isDirty ? 13 : 0);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof DirtyResult)) {
return false;
}
DirtyResult that = (DirtyResult) obj;
return this.isDirty == that.isDirty && Objects.equals(this.newValue, that.newValue);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("isDirty", isDirty)
.add("newValue", newValue)
.toString();
}
}
}

0 comments on commit 5a4b3dd

Please sign in to comment.