In FilesystemValueChecker, use 200 threads and don't waste threads on skipped keys. This yields some noticeable improvements for the wall times of null builds with even a small set of files to be checked for changes.
--
MOS_MIGRATED_REVID=104717653
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 8705593..552419a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -17,7 +17,6 @@
import static com.google.common.hash.Hashing.sha256;
import static com.google.devtools.build.lib.bazel.repository.HttpDownloader.getHash;
-import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -143,19 +142,19 @@
private static final SkyValueDirtinessChecker HTTP_DOWNLOAD_CHECKER =
new SkyValueDirtinessChecker() {
@Override
- @Nullable
- public Optional<SkyValue> maybeCreateNewValue(SkyKey key,
- TimestampGranularityMonitor tsgm) {
+ public boolean applies(SkyKey skyKey) {
+ return skyKey.functionName().equals(HttpDownloadFunction.NAME);
+ }
+
+ @Override
+ public SkyValue createNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
throw new UnsupportedOperationException();
}
@Override
@Nullable
- public DirtyResult maybeCheck(
+ public DirtyResult check(
SkyKey skyKey, SkyValue skyValue, TimestampGranularityMonitor tsgm) {
- if (!skyKey.functionName().equals(HttpDownloadFunction.NAME)) {
- return null;
- }
HttpDownloadValue httpDownloadValue = (HttpDownloadValue) skyValue;
Path path = httpDownloadValue.getPath();
try {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
index 44447b6..ed0c854 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java
@@ -16,7 +16,7 @@
import static com.google.devtools.build.lib.skyframe.SkyFunctions.DIRECTORY_LISTING_STATE;
import static com.google.devtools.build.lib.skyframe.SkyFunctions.FILE_STATE;
-import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Path;
@@ -34,44 +34,40 @@
private DirtinessCheckerUtils() {}
static class FileDirtinessChecker extends SkyValueDirtinessChecker {
- private boolean applies(SkyKey skyKey) {
+ @Override
+ public boolean applies(SkyKey skyKey) {
return skyKey.functionName().equals(FILE_STATE);
}
@Override
@Nullable
- public Optional<SkyValue> maybeCreateNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
- if (!applies(key)) {
- return null;
- }
+ public SkyValue createNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
RootedPath rootedPath = (RootedPath) key.argument();
try {
- return Optional.<SkyValue>of(FileStateValue.create(rootedPath, tsgm));
+ return FileStateValue.create(rootedPath, tsgm);
} catch (InconsistentFilesystemException | IOException e) {
// TODO(bazel-team): An IOException indicates a failure to get a file digest or a symlink
// target, not a missing file. Such a failure really shouldn't happen, so failing early
// may be better here.
- return Optional.<SkyValue>absent();
+ return null;
}
}
}
static class DirectoryDirtinessChecker extends SkyValueDirtinessChecker {
- private boolean applies(SkyKey skyKey) {
+ @Override
+ public boolean applies(SkyKey skyKey) {
return skyKey.functionName().equals(DIRECTORY_LISTING_STATE);
}
@Override
@Nullable
- public Optional<SkyValue> maybeCreateNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
- if (!applies(key)) {
- return null;
- }
+ public SkyValue createNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
RootedPath rootedPath = (RootedPath) key.argument();
try {
- return Optional.<SkyValue>of(DirectoryListingStateValue.create(rootedPath));
+ return DirectoryListingStateValue.create(rootedPath);
} catch (IOException e) {
- return Optional.<SkyValue>absent();
+ return null;
}
}
}
@@ -82,14 +78,15 @@
private final UnionDirtinessChecker checker =
new UnionDirtinessChecker(ImmutableList.of(fdc, ddc));
- protected boolean applies(SkyKey skyKey) {
+ @Override
+ public boolean applies(SkyKey skyKey) {
return fdc.applies(skyKey) || ddc.applies(skyKey);
}
@Override
@Nullable
- public Optional<SkyValue> maybeCreateNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
- return checker.maybeCreateNewValue(key, tsgm);
+ public SkyValue createNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
+ return checker.createNewValue(key, tsgm);
}
}
@@ -101,12 +98,9 @@
}
@Override
- @Nullable
- public DirtyResult maybeCheck(SkyKey key, @Nullable SkyValue oldValue,
- TimestampGranularityMonitor tsgm) {
- return applies(key) && missingDiffPaths.contains(((RootedPath) key.argument()).getRoot())
- ? super.maybeCheck(key, oldValue, tsgm)
- : null;
+ public boolean applies(SkyKey key) {
+ return super.applies(key)
+ && missingDiffPaths.contains(((RootedPath) key.argument()).getRoot());
}
}
@@ -118,29 +112,32 @@
this.dirtinessCheckers = dirtinessCheckers;
}
- @Override
@Nullable
- public Optional<SkyValue> maybeCreateNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
+ private SkyValueDirtinessChecker getChecker(SkyKey key) {
for (SkyValueDirtinessChecker dirtinessChecker : dirtinessCheckers) {
- Optional<SkyValue> newValueMaybe = dirtinessChecker.maybeCreateNewValue(key, tsgm);
- if (newValueMaybe != null) {
- return newValueMaybe;
+ if (dirtinessChecker.applies(key)) {
+ return dirtinessChecker;
}
}
return null;
}
@Override
+ public boolean applies(SkyKey key) {
+ return getChecker(key) != null;
+ }
+
+ @Override
@Nullable
- public DirtyResult maybeCheck(SkyKey key, @Nullable SkyValue oldValue,
+ public SkyValue createNewValue(SkyKey key, TimestampGranularityMonitor tsgm) {
+ return Preconditions.checkNotNull(getChecker(key), key).createNewValue(key, tsgm);
+ }
+
+ @Override
+ @Nullable
+ public DirtyResult check(SkyKey key, @Nullable SkyValue oldValue,
TimestampGranularityMonitor tsgm) {
- for (SkyValueDirtinessChecker dirtinessChecker : dirtinessCheckers) {
- DirtyResult dirtyResult = dirtinessChecker.maybeCheck(key, oldValue, tsgm);
- if (dirtyResult != null) {
- return dirtyResult;
- }
- }
- return null;
+ return Preconditions.checkNotNull(getChecker(key), key).check(key, oldValue, tsgm);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index 35df110..57f64c4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -63,7 +63,7 @@
*/
class FilesystemValueChecker {
- private static final int DIRTINESS_CHECK_THREADS = 50;
+ private static final int DIRTINESS_CHECK_THREADS = 200;
private static final Logger LOG = Logger.getLogger(FilesystemValueChecker.class.getName());
private static final Predicate<SkyKey> ACTION_FILTER =
@@ -311,6 +311,10 @@
};
try (AutoProfiler prof = AutoProfiler.create(elapsedTimeReceiver)) {
for (final SkyKey key : values) {
+ numKeysScanned.incrementAndGet();
+ if (!checker.applies(key)) {
+ continue;
+ }
final SkyValue value = valuesSupplier.get().get(key);
executor.execute(
wrapper.wrap(
@@ -318,13 +322,10 @@
@Override
public void run() {
if (value != null || checkMissingValues) {
- numKeysScanned.incrementAndGet();
- DirtyResult result = checker.maybeCheck(key, value, tsgm);
- if (result != null) {
- numKeysChecked.incrementAndGet();
- if (result.isDirty()) {
- batchResult.add(key, value, result.getNewValue());
- }
+ numKeysChecked.incrementAndGet();
+ DirtyResult result = checker.check(key, value, tsgm);
+ if (result.isDirty()) {
+ batchResult.add(key, value, result.getNewValue());
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java
index 9a24122..8fa15e1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueDirtinessChecker.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.skyframe.SkyKey;
@@ -27,36 +26,26 @@
*/
public abstract class SkyValueDirtinessChecker {
- /**
- * Returns
- * <ul>
- * <li>{@code null}, if the checker can't handle {@code key}.
- * <li>{@code Optional.<SkyValue>absent()} if the checker can handle {@code key} but was unable
- * to create a new value.
- * <li>{@code Optional.<SkyValue>of(v)} if the checker can handle {@code key} and the new value
- * should be {@code v}.
- * </ul>
- */
- @Nullable
- public abstract Optional<SkyValue> maybeCreateNewValue(SkyKey key,
- TimestampGranularityMonitor tsgm);
+ /** Returns {@code true} iff the checker can handle {@code key}. */
+ public abstract boolean applies(SkyKey key);
/**
- * Returns the result of checking whether this key's value is up to date, or null if this
- * dirtiness checker does not apply to this key. If non-null, this answer is assumed to be
- * definitive.
+ * If {@code applies(key)}, returns the new value for {@code key} or {@code null} if the checker
+ * was unable to create a new value.
*/
@Nullable
- public DirtyResult maybeCheck(SkyKey key, @Nullable SkyValue oldValue,
+ public abstract SkyValue createNewValue(SkyKey key, TimestampGranularityMonitor tsgm);
+
+ /**
+ * If {@code applies(key)}, returns the result of checking whether this key's value is up to date.
+ */
+ @Nullable
+ public DirtyResult check(SkyKey key, @Nullable SkyValue oldValue,
TimestampGranularityMonitor tsgm) {
- Optional<SkyValue> newValueMaybe = maybeCreateNewValue(key, tsgm);
- if (newValueMaybe == null) {
- return null;
- }
- if (!newValueMaybe.isPresent()) {
+ SkyValue newValue = createNewValue(key, tsgm);
+ if (newValue == null) {
return DirtyResult.dirty(oldValue);
}
- SkyValue newValue = Preconditions.checkNotNull(newValueMaybe.get(), key);
return newValue.equals(oldValue)
? DirtyResult.notDirty(oldValue)
: DirtyResult.dirtyWithNewValue(oldValue, newValue);
