Replace UnixGlob's package glob prefetching functionality with an
optional method in FileSystem. Custom FileSystem implementations can
use this to provide their own implementation of glob prefetching.
--
MOS_MIGRATED_REVID=140736304
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index 3c960df..58321cc 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -24,6 +24,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -35,7 +36,6 @@
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.testutil.ManualClock;
-import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.BlazeClock;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Dirent;
@@ -56,10 +56,8 @@
import java.io.IOException;
import java.util.Collection;
import java.util.Map;
+import java.util.Set;
import java.util.UUID;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -760,255 +758,74 @@
}
}
- @Test
- public void testGlobsHappenInParallel() throws Exception {
- scratch.file(
- "foo/BUILD",
- "load('//foo:my_library.bzl', 'my_library')",
- "[sh_library(name = x + '-matched') for x in glob(['bar/*'], exclude_directories = 0)]",
- "cc_library(name = 'cc', srcs = glob(['cc/*']))",
- "my_library(name = 'my', srcs = glob(['sh/*']))");
- scratch.file(
- "foo/my_library.bzl",
- "def my_library(name = None, srcs = []):",
- " native.sh_library(name = name, srcs = srcs, deps = native.glob(['inner/*']))");
- scratch.file("foo/bar/1");
- Path barPath = scratch.file("foo/bar/2").getParentDirectory();
- Path ccPath = scratch.file("foo/cc/src.file").getParentDirectory();
- Path shPath = scratch.dir("foo/sh");
- Path innerPath = scratch.dir("foo/inner");
- PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
- packageCacheOptions.defaultVisibility = ConstantRuleVisibility.PUBLIC;
- packageCacheOptions.showLoadingProgress = true;
- packageCacheOptions.globbingThreads = 7;
- packageCacheOptions.maxDirectoriesToEagerlyVisitInGlobbing = 10;
- getSkyframeExecutor()
- .preparePackageLoading(
- new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)),
- packageCacheOptions,
- "",
- UUID.randomUUID(),
- ImmutableMap.<String, String>of(),
- new TimestampGranularityMonitor(BlazeClock.instance()));
-
- SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
- final CountDownLatch allDirsRequested = new CountDownLatch(4);
- Listener synchronizeListener =
- new Listener() {
- @Override
- public Object accept(Path path, FileOp op, Order order) throws IOException {
- if (op == FileOp.READDIR && order == Order.BEFORE) {
- allDirsRequested.countDown();
- try {
- assertThat(
- allDirsRequested.await(
- TestUtils.WAIT_TIMEOUT_MILLISECONDS, TimeUnit.MILLISECONDS))
- .isTrue();
- } catch (InterruptedException e) {
- throw new IllegalStateException(e);
- }
- }
- return NO_RESULT_MARKER;
- }
- };
- fs.setCustomOverride(barPath, synchronizeListener);
- fs.setCustomOverride(ccPath, synchronizeListener);
- fs.setCustomOverride(shPath, synchronizeListener);
- fs.setCustomOverride(innerPath, synchronizeListener);
- PackageValue value = validPackage(skyKey);
- assertFalse(value.getPackage().containsErrors());
- assertThat(value.getPackage().getTarget("bar/1-matched").getName()).isEqualTo("bar/1-matched");
- assertThat(value.getPackage().getTarget("cc/src.file")).isNotNull();
- assertThat(
- (Iterable<?>)
- value
- .getPackage()
- .getTarget("my")
- .getAssociatedRule()
- .getAttributeContainer()
- .getAttr("srcs"))
- .isEmpty();
- }
-
- @Test
- public void testGlobsDontHappenInParallel() throws Exception {
- scratch.file(
- "foo/BUILD",
- "load('//foo:my_library.bzl', 'my_library')",
- "[sh_library(name = x + '-matched') for x in glob(['bar/*'], exclude_directories = 0)]",
- "cc_library(name = 'cc', srcs = glob(['cc/*']))",
- "my_library(name = 'my', srcs = glob(['sh/*']))");
- scratch.file(
- "foo/my_library.bzl",
- "def my_library(name = None, srcs = []):",
- " native.sh_library(name = name, srcs = srcs, deps = native.glob(['inner/*']))");
- scratch.file("foo/bar/1");
- Path barPath = scratch.file("foo/bar/2").getParentDirectory();
- Path ccPath = scratch.file("foo/cc/src.file").getParentDirectory();
- Path shPath = scratch.dir("foo/sh");
- Path innerPath = scratch.dir("foo/inner");
- PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
- packageCacheOptions.defaultVisibility = ConstantRuleVisibility.PUBLIC;
- packageCacheOptions.showLoadingProgress = true;
- packageCacheOptions.globbingThreads = 7;
- packageCacheOptions.maxDirectoriesToEagerlyVisitInGlobbing = -1;
- getSkyframeExecutor()
- .preparePackageLoading(
- new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)),
- packageCacheOptions,
- "",
- UUID.randomUUID(),
- ImmutableMap.<String, String>of(),
- new TimestampGranularityMonitor(BlazeClock.instance()));
-
- SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
- final AtomicBoolean atLeastOneUnfinishedRequest = new AtomicBoolean(false);
- final CountDownLatch allDirsRequested = new CountDownLatch(4);
- Listener synchronizeListener =
- new Listener() {
- @Override
- public Object accept(Path path, FileOp op, Order order) throws IOException {
- if (op == FileOp.READDIR && order == Order.BEFORE) {
- allDirsRequested.countDown();
- try {
- if (!allDirsRequested.await(1, TimeUnit.SECONDS)) {
- atLeastOneUnfinishedRequest.set(true);
- }
- } catch (InterruptedException e) {
- throw new IllegalStateException(e);
- }
- }
- return NO_RESULT_MARKER;
- }
- };
- fs.setCustomOverride(barPath, synchronizeListener);
- fs.setCustomOverride(ccPath, synchronizeListener);
- fs.setCustomOverride(shPath, synchronizeListener);
- fs.setCustomOverride(innerPath, synchronizeListener);
- PackageValue value = validPackage(skyKey);
- assertFalse(value.getPackage().containsErrors());
- assertThat(value.getPackage().getTarget("bar/1-matched").getName()).isEqualTo("bar/1-matched");
- assertThat(value.getPackage().getTarget("cc/src.file")).isNotNull();
- assertThat(
- (Iterable<?>)
- value
- .getPackage()
- .getTarget("my")
- .getAssociatedRule()
- .getAttributeContainer()
- .getAttr("srcs"))
- .isEmpty();
- assertThat(atLeastOneUnfinishedRequest.get()).isTrue();
- }
-
private static class CustomInMemoryFs extends InMemoryFileSystem {
- private final Map<Path, Listener> customOverrides = Maps.newHashMap();
+ private abstract static class FileStatusOrException {
+ abstract FileStatus get() throws IOException;
+
+ private static class ExceptionImpl extends FileStatusOrException {
+ private final IOException exn;
+
+ private ExceptionImpl(IOException exn) {
+ this.exn = exn;
+ }
+
+ @Override
+ FileStatus get() throws IOException {
+ throw exn;
+ }
+ }
+
+ private static class FileStatusImpl extends FileStatusOrException {
+
+ @Nullable
+ private final FileStatus fileStatus;
+
+ private FileStatusImpl(@Nullable FileStatus fileStatus) {
+ this.fileStatus = fileStatus;
+ }
+
+ @Override
+ @Nullable
+ FileStatus get() {
+ return fileStatus;
+ }
+ }
+ }
+
+ private Map<Path, FileStatusOrException> stubbedStats = Maps.newHashMap();
+ private Set<Path> makeUnreadableAfterReaddir = Sets.newHashSet();
public CustomInMemoryFs(ManualClock manualClock) {
super(manualClock);
}
- public void stubStat(final Path targetPath, @Nullable final FileStatus stubbedResult) {
- setCustomOverride(
- targetPath,
- new Listener() {
- @Override
- public Object accept(Path path, FileOp op, Order order) {
- if (targetPath.equals(path) && op == FileOp.STAT && order == Order.BEFORE) {
- return stubbedResult;
- } else {
- return NO_RESULT_MARKER;
- }
- }
- });
+ public void stubStat(Path path, @Nullable FileStatus stubbedResult) {
+ stubbedStats.put(path, new FileStatusOrException.FileStatusImpl(stubbedResult));
}
- public void stubStatError(final Path targetPath, final IOException stubbedResult) {
- setCustomOverride(
- targetPath,
- new Listener() {
- @Override
- public Object accept(Path path, FileOp op, Order order) throws IOException {
- if (targetPath.equals(path) && op == FileOp.STAT && order == Order.BEFORE) {
- throw stubbedResult;
- } else {
- return NO_RESULT_MARKER;
- }
- }
- });
- }
-
- void setCustomOverride(Path path, Listener listener) {
- customOverrides.put(path, listener);
+ public void stubStatError(Path path, IOException stubbedResult) {
+ stubbedStats.put(path, new FileStatusOrException.ExceptionImpl(stubbedResult));
}
@Override
public FileStatus stat(Path path, boolean followSymlinks) throws IOException {
- Listener listener = customOverrides.get(path);
- if (listener != null) {
- Object status = listener.accept(path, FileOp.STAT, Order.BEFORE);
- if (status != NO_RESULT_MARKER) {
- return (FileStatus) status;
- }
+ if (stubbedStats.containsKey(path)) {
+ return stubbedStats.get(path).get();
}
- FileStatus fileStatus = super.stat(path, followSymlinks);
- if (listener != null) {
- Object status = listener.accept(path, FileOp.STAT, Order.AFTER);
- if (status != NO_RESULT_MARKER) {
- return (FileStatus) status;
- }
- }
- return fileStatus;
+ return super.stat(path, followSymlinks);
}
- public void scheduleMakeUnreadableAfterReaddir(final Path targetPath) {
- setCustomOverride(
- targetPath,
- new Listener() {
- @Override
- public Object accept(Path path, FileOp op, Order order) throws IOException {
- if (targetPath.equals(path) && op == FileOp.READDIR && order == Order.AFTER) {
- targetPath.setReadable(false);
- }
- return NO_RESULT_MARKER;
- }
- });
+ public void scheduleMakeUnreadableAfterReaddir(Path path) {
+ makeUnreadableAfterReaddir.add(path);
}
- @SuppressWarnings("unchecked")
@Override
public Collection<Dirent> readdir(Path path, boolean followSymlinks) throws IOException {
- Listener listener = customOverrides.get(path);
- if (listener != null) {
- Object status = listener.accept(path, FileOp.READDIR, Order.BEFORE);
- if (status != NO_RESULT_MARKER) {
- return (Collection<Dirent>) status;
- }
- }
Collection<Dirent> result = super.readdir(path, followSymlinks);
- if (listener != null) {
- Object status = listener.accept(path, FileOp.READDIR, Order.AFTER);
- if (status != NO_RESULT_MARKER) {
- return (Collection<Dirent>) status;
- }
+ if (makeUnreadableAfterReaddir.contains(path)) {
+ path.setReadable(false);
}
return result;
}
}
-
- private static final Object NO_RESULT_MARKER = new Object();
-
- private enum Order {
- BEFORE,
- AFTER
- }
-
- private enum FileOp {
- STAT,
- READDIR
- }
-
- private interface Listener {
- Object accept(Path path, FileOp op, Order order) throws IOException;
- }
}
