Add the option --experimental_max_directories_to_eagerly_visit_in_globbing. The first legacy glob that a package requires will, if this option is enabled, cause up to that many directories to be eagerly visited by a glob(['**']). The results are thrown away for memory reasons.
--
MOS_MIGRATED_REVID=135148361
diff --git a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
index 694601d..5259385 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
@@ -26,7 +26,6 @@
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.UnixGlob;
-
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
@@ -39,6 +38,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
/**
@@ -76,29 +76,37 @@
* System call caching layer.
*/
private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
+ private final int maxDirectoriesToEagerlyVisit;
/**
* The thread pool for glob evaluation.
*/
private final ThreadPoolExecutor globExecutor;
+ private final AtomicBoolean globalStarted = new AtomicBoolean(false);
/**
* Create a glob expansion cache.
- * @param packageDirectory globs will be expanded relatively to this
- * directory.
+ *
+ * @param packageDirectory globs will be expanded relatively to this directory.
* @param packageId the name of the package this cache belongs to.
* @param locator the package locator.
* @param globExecutor thread pool for glob evaluation.
+ * @param maxDirectoriesToEagerlyVisit the number of directories to eagerly traverse on the first
+ * glob for a given package, in order to warm the filesystem. -1 means do no eager traversal.
+ * See {@code PackageCacheOptions#maxDirectoriesToEagerlyVisitInGlobbing}.
*/
- public GlobCache(final Path packageDirectory,
- final PackageIdentifier packageId,
- final CachingPackageLocator locator,
- AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls,
- ThreadPoolExecutor globExecutor) {
+ public GlobCache(
+ final Path packageDirectory,
+ final PackageIdentifier packageId,
+ final CachingPackageLocator locator,
+ AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls,
+ ThreadPoolExecutor globExecutor,
+ int maxDirectoriesToEagerlyVisit) {
this.packageDirectory = Preconditions.checkNotNull(packageDirectory);
this.packageId = Preconditions.checkNotNull(packageId);
this.globExecutor = Preconditions.checkNotNull(globExecutor);
this.syscalls = syscalls == null ? new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS) : syscalls;
+ this.maxDirectoriesToEagerlyVisit = maxDirectoriesToEagerlyVisit;
Preconditions.checkNotNull(locator);
childDirectoryPredicate = new Predicate<Path>() {
@@ -129,6 +137,18 @@
throws BadGlobException {
Future<List<Path>> cached = globCache.get(Pair.of(pattern, excludeDirs));
if (cached == null) {
+ if (maxDirectoriesToEagerlyVisit > -1
+ && !globalStarted.getAndSet(true)
+ && !pattern.startsWith("**")) {
+ UnixGlob.forPath(packageDirectory)
+ .setMaxDirectoriesToEagerlyVisit(maxDirectoriesToEagerlyVisit)
+ .addPattern("**")
+ .setExcludeDirectories(true)
+ .setDirectoryFilter(childDirectoryPredicate)
+ .setThreadPool(globExecutor)
+ .setFilesystemCalls(syscalls)
+ .globAsync(true);
+ }
cached = safeGlobUnsorted(pattern, excludeDirs);
setGlobPaths(pattern, excludeDirs, cached);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 86406f4..52cb946 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -334,6 +334,8 @@
private final ThreadPoolExecutor threadPool;
private Map<String, String> platformSetRegexps;
+ private int maxDirectoriesToEagerlyVisitInGlobbing;
+
private final ImmutableList<EnvironmentExtension> environmentExtensions;
private final ImmutableMap<String, PackageArgument<?>> packageArguments;
@@ -429,6 +431,10 @@
threadPool.setMaximumPoolSize(globbingThreads);
}
+ public void setMaxDirectoriesToEagerlyVisitInGlobbing(
+ int maxDirectoriesToEagerlyVisitInGlobbing) {
+ this.maxDirectoriesToEagerlyVisitInGlobbing = maxDirectoriesToEagerlyVisitInGlobbing;
+ }
/**
* Returns the immutable, unordered set of names of all the known rule
@@ -1422,8 +1428,14 @@
Path packageDirectory,
PackageIdentifier packageId,
CachingPackageLocator locator) {
- return createLegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
- threadPool));
+ return createLegacyGlobber(
+ new GlobCache(
+ packageDirectory,
+ packageId,
+ locator,
+ syscalls,
+ threadPool,
+ maxDirectoriesToEagerlyVisitInGlobbing));
}
/** Returns a new {@link LegacyGlobber}. */
@@ -1440,8 +1452,15 @@
Path packageDirectory,
PackageIdentifier packageId,
CachingPackageLocator locator) {
- return new LegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
- threadPool), /*sort=*/ false);
+ return new LegacyGlobber(
+ new GlobCache(
+ packageDirectory,
+ packageId,
+ locator,
+ syscalls,
+ threadPool,
+ maxDirectoriesToEagerlyVisitInGlobbing),
+ /*sort=*/ false);
}
@Nullable
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java
index 0664229..f22fceb 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java
@@ -110,6 +110,17 @@
help = "Number of threads to use for glob evaluation.")
public int globbingThreads;
+ @Option(
+ name = "experimental_max_directories_to_eagerly_visit_in_globbing",
+ defaultValue = "-1",
+ category = "undocumented",
+ help =
+ "If non-negative, the first time a glob is evaluated in a package, the subdirectories of "
+ + "the package will be traversed in order to warm filesystem caches and compensate for "
+ + "lack of parallelism in globbing. At most this many directories will be visited."
+ )
+ public int maxDirectoriesToEagerlyVisitInGlobbing;
+
@Option(name = "fetch",
defaultValue = "true",
category = "undocumented",
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 c87db1e..9d4d72d 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
@@ -920,6 +920,8 @@
syscalls.set(newPerBuildSyscallCache(packageCacheOptions.globbingThreads));
this.pkgFactory.setGlobbingThreads(packageCacheOptions.globbingThreads);
+ this.pkgFactory.setMaxDirectoriesToEagerlyVisitInGlobbing(
+ packageCacheOptions.maxDirectoriesToEagerlyVisitInGlobbing);
checkPreprocessorFactory();
emittedEventState.clear();
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
index 1ea067d..82b5e55 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
@@ -35,7 +35,6 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.util.Preconditions;
-
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
@@ -45,6 +44,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Pattern;
@@ -68,9 +68,10 @@
FilesystemCalls syscalls,
ThreadPoolExecutor threadPool)
throws IOException, InterruptedException {
- GlobVisitor visitor = (threadPool == null)
- ? new GlobVisitor(checkForInterruption)
- : new GlobVisitor(threadPool, checkForInterruption);
+ GlobVisitor visitor =
+ (threadPool == null)
+ ? new GlobVisitor(checkForInterruption)
+ : new GlobVisitor(threadPool, checkForInterruption, -1);
return visitor.glob(base, patterns, excludeDirectories, dirPred, syscalls);
}
@@ -81,22 +82,26 @@
boolean checkForInterruption,
FilesystemCalls syscalls,
ThreadPoolExecutor threadPool) throws IOException, InterruptedException {
- GlobVisitor visitor = (threadPool == null)
- ? new GlobVisitor(checkForInterruption)
- : new GlobVisitor(threadPool, checkForInterruption);
+ GlobVisitor visitor =
+ (threadPool == null)
+ ? new GlobVisitor(checkForInterruption)
+ : new GlobVisitor(threadPool, checkForInterruption, -1);
visitor.glob(base, patterns, excludeDirectories, dirPred, syscalls);
return visitor.getNumGlobTasksForTesting();
}
- private static Future<List<Path>> globAsyncInternal(Path base, Collection<String> patterns,
- boolean excludeDirectories,
- Predicate<Path> dirPred,
- FilesystemCalls syscalls,
- boolean checkForInterruption,
- ThreadPoolExecutor threadPool) {
+ private static Future<List<Path>> globAsyncInternal(
+ Path base,
+ Collection<String> patterns,
+ boolean excludeDirectories,
+ Predicate<Path> dirPred,
+ FilesystemCalls syscalls,
+ boolean checkForInterruption,
+ ThreadPoolExecutor threadPool,
+ int maxDirectoriesToEagerlyVisit) {
Preconditions.checkNotNull(threadPool, "%s %s", base, patterns);
- return new GlobVisitor(threadPool, checkForInterruption).globAsync(
- base, patterns, excludeDirectories, dirPred, syscalls);
+ return new GlobVisitor(threadPool, checkForInterruption, maxDirectoriesToEagerlyVisit)
+ .globAsync(base, patterns, excludeDirectories, dirPred, syscalls);
}
/**
@@ -304,6 +309,7 @@
private ThreadPoolExecutor threadPool;
private AtomicReference<? extends FilesystemCalls> syscalls =
new AtomicReference<>(DEFAULT_SYSCALLS);
+ private int maxDirectoriesToEagerlyVisit = -1;
/**
* Creates a glob builder with the given base path.
@@ -384,6 +390,11 @@
return this;
}
+ public Builder setMaxDirectoriesToEagerlyVisit(int maxDirectoriesToEagerlyVisit) {
+ this.maxDirectoriesToEagerlyVisit = maxDirectoriesToEagerlyVisit;
+ return this;
+ }
+
/**
* Executes the glob.
*/
@@ -421,8 +432,15 @@
* @param checkForInterrupt if the returned future may throw InterruptedException.
*/
public Future<List<Path>> globAsync(boolean checkForInterrupt) {
- return globAsyncInternal(base, patterns, excludeDirectories, pathFilter, syscalls.get(),
- checkForInterrupt, threadPool);
+ return globAsyncInternal(
+ base,
+ patterns,
+ excludeDirectories,
+ pathFilter,
+ syscalls.get(),
+ checkForInterrupt,
+ threadPool,
+ maxDirectoriesToEagerlyVisit);
}
}
@@ -489,15 +507,21 @@
private final AtomicLong totalOps = new AtomicLong(0);
private final AtomicLong pendingOps = new AtomicLong(0);
private final AtomicReference<IOException> failure = new AtomicReference<>();
+ private final int maxDirectoriesToEagerlyVisit;
+ private final AtomicInteger visitedDirectories = new AtomicInteger(0);
private volatile boolean canceled = false;
- public GlobVisitor(ThreadPoolExecutor executor, boolean failFastOnInterrupt) {
+ GlobVisitor(
+ ThreadPoolExecutor executor,
+ boolean failFastOnInterrupt,
+ int maxDirectoriesToEagerlyVisit) {
this.executor = executor;
this.result = new GlobFuture(this, failFastOnInterrupt);
+ this.maxDirectoriesToEagerlyVisit = maxDirectoriesToEagerlyVisit;
}
- public GlobVisitor(boolean failFastOnInterrupt) {
- this(null, failFastOnInterrupt);
+ GlobVisitor(boolean failFastOnInterrupt) {
+ this(null, failFastOnInterrupt, -1);
}
/**
@@ -534,11 +558,23 @@
}
/**
+ * Whether or not to store the results of this glob. If this glob is being done purely to warm
+ * the filesystem, we do not store the results, since it would take unnecessary memory.
+ */
+ private boolean storeGlobResults() {
+ return maxDirectoriesToEagerlyVisit == -1;
+ }
+
+ /**
* Same as {@link #glob}, except does so asynchronously and returns a {@link Future} for the
* result.
*/
- public Future<List<Path>> globAsync(Path base, Collection<String> patterns,
- boolean excludeDirectories, Predicate<Path> dirPred, FilesystemCalls syscalls) {
+ public Future<List<Path>> globAsync(
+ Path base,
+ Collection<String> patterns,
+ boolean excludeDirectories,
+ Predicate<Path> dirPred,
+ FilesystemCalls syscalls) {
FileStatus baseStat = syscalls.statNullable(base, Symlinks.FOLLOW);
if (baseStat == null || patterns.isEmpty()) {
@@ -744,7 +780,7 @@
}
if (idx == context.patternParts.length) { // Base case.
- if (!(context.excludeDirectories && baseIsDir)) {
+ if (storeGlobResults() && !(context.excludeDirectories && baseIsDir)) {
results.add(base);
}
@@ -756,6 +792,10 @@
return;
}
+ if (maxDirectoriesToEagerlyVisit > -1
+ && visitedDirectories.incrementAndGet() > maxDirectoriesToEagerlyVisit) {
+ return;
+ }
final String pattern = context.patternParts[idx];
// ** is special: it can match nothing at all.
@@ -803,7 +843,7 @@
context.queueGlob(child, childIsDir, idx + 1);
} else {
// Instead of using an async call, just repeat the base case above.
- if (idx + 1 == context.patternParts.length) {
+ if (storeGlobResults() && idx + 1 == context.patternParts.length) {
results.add(child);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
index 7285ede..4d120c1 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
@@ -24,18 +24,16 @@
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
-
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/**
* Tests for {@link GlobCache}
@@ -89,20 +87,26 @@
scratch.file("isolated/sub/sub.js",
"# this is sub/sub.js");
- cache = new GlobCache(packageDirectory, PackageIdentifier.createInMainRepo("isolated"),
- new CachingPackageLocator() {
- @Override
- public Path getBuildFileForPackage(PackageIdentifier packageId) {
- String packageName = packageId.getPackageFragment().getPathString();
- if (packageName.equals("isolated")) {
- return scratch.resolve("isolated/BUILD");
- } else if (packageName.equals("isolated/sub")) {
- return scratch.resolve("isolated/sub/BUILD");
- } else {
- return null;
- }
- }
- }, null, TestUtils.getPool());
+ cache =
+ new GlobCache(
+ packageDirectory,
+ PackageIdentifier.createInMainRepo("isolated"),
+ new CachingPackageLocator() {
+ @Override
+ public Path getBuildFileForPackage(PackageIdentifier packageId) {
+ String packageName = packageId.getPackageFragment().getPathString();
+ if (packageName.equals("isolated")) {
+ return scratch.resolve("isolated/BUILD");
+ } else if (packageName.equals("isolated/sub")) {
+ return scratch.resolve("isolated/sub/BUILD");
+ } else {
+ return null;
+ }
+ }
+ },
+ null,
+ TestUtils.getPool(),
+ -1);
}
@After
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index e53690e..505a845 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -120,7 +120,8 @@
packageId,
getPackageLocator(),
null,
- TestUtils.getPool());
+ TestUtils.getPool(),
+ -1);
LegacyGlobber globber = PackageFactory.createLegacyGlobber(globCache);
Package externalPkg =
factory.newExternalPackageBuilder(
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java
index 5d8c1da..6db8486 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java
@@ -42,9 +42,6 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
-
-import org.junit.Before;
-
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Collection;
@@ -53,6 +50,7 @@
import java.util.concurrent.Semaphore;
import java.util.logging.Handler;
import java.util.logging.LogRecord;
+import org.junit.Before;
/**
* Base class for PackageFactory tests.
@@ -113,7 +111,8 @@
pkg.getPackageIdentifier(),
PackageFactoryApparatus.createEmptyLocator(),
null,
- TestUtils.getPool());
+ TestUtils.getPool(),
+ -1);
assertThat(globCache.globUnsorted(include, exclude, false)).containsExactlyElementsIn(expected);
}
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 b5d8c16..9175145 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,7 +24,6 @@
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;
@@ -36,6 +35,7 @@
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,8 +56,10 @@
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;
@@ -761,74 +763,255 @@
}
}
+ @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 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();
+ private final Map<Path, Listener> customOverrides = Maps.newHashMap();
public CustomInMemoryFs(ManualClock manualClock) {
super(manualClock);
}
- public void stubStat(Path path, @Nullable FileStatus stubbedResult) {
- stubbedStats.put(path, new FileStatusOrException.FileStatusImpl(stubbedResult));
+ 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 stubStatError(Path path, IOException stubbedResult) {
- stubbedStats.put(path, new FileStatusOrException.ExceptionImpl(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);
}
@Override
public FileStatus stat(Path path, boolean followSymlinks) throws IOException {
- if (stubbedStats.containsKey(path)) {
- return stubbedStats.get(path).get();
+ 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;
+ }
}
- return super.stat(path, followSymlinks);
+ 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;
}
- public void scheduleMakeUnreadableAfterReaddir(Path path) {
- makeUnreadableAfterReaddir.add(path);
+ 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;
+ }
+ });
}
+ @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 (makeUnreadableAfterReaddir.contains(path)) {
- path.setReadable(false);
+ if (listener != null) {
+ Object status = listener.accept(path, FileOp.READDIR, Order.AFTER);
+ if (status != NO_RESULT_MARKER) {
+ return (Collection<Dirent>) status;
+ }
}
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;
+ }
}
