diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java index 30e18cadbf6a8d..29324fea93c0e7 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java @@ -24,6 +24,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; import com.google.common.io.CharStreams; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.Artifact; @@ -886,6 +889,72 @@ Collection extractInclusions( return ImmutableList.copyOf(inclusions); } + /** + * Extracts all inclusions from a given source file. + * + * @param file the file to parse & extract inclusions from + * @param actionExecutionContext Services in the scope of the action, like the stream to which + * scanning messages are printed + * @return a new set of inclusions, normalized to the cache + */ + ListenableFuture> extractInclusionsAsync( + Artifact file, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Artifact grepIncludes, + @Nullable SpawnIncludeScanner remoteIncludeScanner, + boolean isOutputFile) + throws IOException { + ListenableFuture> inclusions; + if (remoteIncludeScanner != null + && remoteIncludeScanner.shouldParseRemotely(file, actionExecutionContext)) { + inclusions = + remoteIncludeScanner.extractInclusionsAsync( + file, + actionExecutionMetadata, + actionExecutionContext, + grepIncludes, + getFileType(), + isOutputFile); + } else { + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.SCANNER, file.getExecPathString())) { + inclusions = + Futures.immediateFuture( + extractInclusions( + FileSystemUtils.readContent(actionExecutionContext.getInputPath(file)))); + } catch (IOException e) { + if (remoteIncludeScanner != null) { + logger.log( + Level.WARNING, + "Falling back on remote parsing of " + actionExecutionContext.getInputPath(file), + e); + inclusions = + remoteIncludeScanner.extractInclusionsAsync( + file, + actionExecutionMetadata, + actionExecutionContext, + grepIncludes, + getFileType(), + isOutputFile); + } else { + throw e; + } + } + } + if (hints != null) { + return Futures.transform( + inclusions, + (c) -> { + // Ugly, but saves doing another copy. + c.addAll(hints.getHintedInclusions(file)); + return c; + }, + MoreExecutors.directExecutor()); + } + return inclusions; + } + /** * Returns type of the scanned file. * diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplierImpl.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplierImpl.java index 4957120ff0b397..f73fcf9159b44f 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplierImpl.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplierImpl.java @@ -18,6 +18,7 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -76,7 +77,7 @@ public boolean equals(Object other) { * Cache of include scan results mapping source paths to sets of scanned inclusions. Shared by all * scanner instances. */ - private final ConcurrentMap> includeParseCache = + private final ConcurrentMap>> includeParseCache = new ConcurrentHashMap<>(); /** Map of grepped include files from input (.cc or .h) to a header-grepped file. */ @@ -84,6 +85,7 @@ public boolean equals(Object other) { private final Supplier spawnIncludeScannerSupplier; private final Path execRoot; + private final boolean useAsyncIncludeScanner; /** Cache of include scanner instances mapped by include-path hashes. */ private final LoadingCache scanners = @@ -102,7 +104,8 @@ public IncludeScanner load(IncludeScannerParams key) { directories.getOutputPath(execRoot.getBaseName()), execRoot, artifactFactory, - spawnIncludeScannerSupplier); + spawnIncludeScannerSupplier, + useAsyncIncludeScanner); } }); @@ -111,13 +114,15 @@ public IncludeScannerSupplierImpl( ExecutorService includePool, ArtifactFactory artifactFactory, Supplier spawnIncludeScannerSupplier, - Path execRoot) { + Path execRoot, + boolean useAsyncIncludeScanner) { this.directories = directories; this.includePool = includePool; this.artifactFactory = artifactFactory; this.spawnIncludeScannerSupplier = spawnIncludeScannerSupplier; this.execRoot = execRoot; this.pathCache = new PathExistenceCache(execRoot, artifactFactory); + this.useAsyncIncludeScanner = useAsyncIncludeScanner; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java index cd4ff724e5928d..76d2ca8f327a91 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.actions.ActionContext; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -148,15 +149,19 @@ public static final class SwigIncludeScanningContextImpl implements SwigIncludeS private final CommandEnvironment env; private final Supplier spawnScannerSupplier; private final Supplier includePool; - private final ConcurrentMap> cache = new ConcurrentHashMap<>(); + private final ConcurrentMap>> cache = + new ConcurrentHashMap<>(); + private final boolean useAsyncIncludeScanner; SwigIncludeScanningContextImpl( CommandEnvironment env, Supplier spawnScannerSupplier, - Supplier includePool) { + Supplier includePool, + boolean useAsyncIncludeScanner) { this.env = env; this.spawnScannerSupplier = spawnScannerSupplier; this.includePool = includePool; + this.useAsyncIncludeScanner = useAsyncIncludeScanner; } @Override @@ -177,7 +182,8 @@ public void extractSwigIncludes( swigIncludePaths, env.getDirectories(), env.getSkyframeBuildView().getArtifactFactory(), - env.getExecRoot()); + env.getExecRoot(), + useAsyncIncludeScanner); ImmutableMap.Builder pathToLegalOutputArtifact = ImmutableMap.builder(); for (Artifact path : legalOutputPaths) { @@ -238,7 +244,8 @@ public IncludeScanningActionContextProvider( this.strategies = ImmutableList.of( new CppIncludeExtractionContextImpl(env), - new SwigIncludeScanningContextImpl(env, spawnScannerSupplier, () -> includePool), + new SwigIncludeScanningContextImpl( + env, spawnScannerSupplier, () -> includePool, options.useAsyncIncludeScanner), new CppIncludeScanningContextImpl(() -> includeScannerSupplier)); env.getEventBus().register(this); @@ -270,8 +277,8 @@ public void executionPhaseStarting( @Override public void executorCreated(Iterable usedContexts) throws ExecutorInitException { - int threads = buildRequest.getOptions(IncludeScanningOptions.class) - .includeScanningParallelism; + IncludeScanningOptions options = buildRequest.getOptions(IncludeScanningOptions.class); + int threads = options.includeScanningParallelism; if (threads > 0) { log.info( String.format("Include scanning configured to use a pool with %d threads", threads)); @@ -286,11 +293,11 @@ public void executorCreated(Iterable usedContexts) throws Executo includePool, env.getSkyframeBuildView().getArtifactFactory(), spawnScannerSupplier, - env.getExecRoot()); + env.getExecRoot(), + options.useAsyncIncludeScanner); spawnScannerSupplier.get().setOutputService(env.getOutputService()); - spawnScannerSupplier.get().setInMemoryOutput( - buildRequest.getOptions(IncludeScanningOptions.class).inMemoryIncludesFiles); + spawnScannerSupplier.get().setInMemoryOutput(options.inMemoryIncludesFiles); } } } diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningOptions.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningOptions.java index 9fc65b94553ad3..56cdec42691175 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningOptions.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningOptions.java @@ -100,4 +100,16 @@ public ParallelismConverter() throws OptionsParsingException { + " \"auto\" means to use a reasonable value derived from the machine's hardware" + " profile (e.g. the number of processors).") public int includeScanningParallelism; + + @Option( + name = "experimental_async_include_scanner", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = { + OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION, + OptionEffectTag.EXECUTION, + OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS + }, + defaultValue = "false", + help = "Switches to a new implementation of include scanning based on futures.") + public boolean useAsyncIncludeScanner; } diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java index d14bbb209872b2..4d1e2bbd178d6c 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java @@ -15,11 +15,14 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.Artifact; @@ -53,6 +56,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; /** @@ -270,7 +274,7 @@ static LocateOnPathResult createNotFound(boolean viewedIllegalOutputFile) { * Externally-scoped cache of file path => parsed inclusion set mappings. Saves us from having to * parse files more than once, and can be shared by scanners with different search paths. */ - private final ConcurrentMap> fileParseCache; + private final ConcurrentMap>> fileParseCache; private final IncludeParser parser; @@ -304,6 +308,8 @@ static LocateOnPathResult createNotFound(boolean viewedIllegalOutputFile) { private final ExecutorService includePool; + private final boolean useAsyncIncludeScanner; + // We are using this Random just for shuffling, so keep the order deterministic by hardcoding // the seed. private static final Random CONSTANT_SEED_RANDOM = new Random(88); @@ -319,14 +325,15 @@ static LocateOnPathResult createNotFound(boolean viewedIllegalOutputFile) { LegacyIncludeScanner( IncludeParser parser, ExecutorService includePool, - ConcurrentMap> cache, + ConcurrentMap>> cache, PathExistenceCache pathCache, List quoteIncludePaths, List includePaths, Path outputPath, Path execRoot, ArtifactFactory artifactFactory, - Supplier spawnIncludeScannerSupplier) { + Supplier spawnIncludeScannerSupplier, + boolean useAsyncIncludeScanner) { this.parser = parser; this.includePool = includePool; this.fileParseCache = cache; @@ -344,6 +351,7 @@ static LocateOnPathResult createNotFound(boolean viewedIllegalOutputFile) { this.includeRootFragment = outputPathFragment.getRelative(BlazeDirectories.RELATIVE_INCLUDE_DIR); this.absoluteRoot = Root.absoluteRoot(execRoot.getFileSystem()); + this.useAsyncIncludeScanner = useAsyncIncludeScanner; } /** @@ -431,7 +439,11 @@ public ListenableFuture processAsync( throws IOException, ExecException, InterruptedException { ImmutableSet pathHints = prepare(actionExecutionContext.getEnvironmentForDiscoveringInputs()); - IncludeVisitor visitor = new IncludeVisitor(includeScanningHeaderData.getModularHeaders()); + IncludeVisitor visitor; + visitor = + useAsyncIncludeScanner + ? new AsyncIncludeVisitor(includeScanningHeaderData.getModularHeaders()) + : new LegacyIncludeVisitor(includeScanningHeaderData.getModularHeaders()); return visitor.processInternal( mainSource, sources, @@ -485,15 +497,29 @@ private boolean isIncPath(PathFragment path) { return path.startsWith(includeRootFragment) && !path.equals(includeRootFragment); } + private interface IncludeVisitor { + ListenableFuture processInternal( + Artifact mainSource, + Collection sources, + IncludeScanningHeaderData includeScanningHeaderData, + List cmdlineIncludes, + Set includes, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Artifact grepIncludes, + ImmutableSet pathHints) + throws InterruptedException, IOException, ExecException; + } + /** * Implements a potentially parallel traversal over source files using a thread pool shared across * different IncludeScanner instances. */ - private class IncludeVisitor extends AbstractQueueVisitor { + private class LegacyIncludeVisitor extends AbstractQueueVisitor implements IncludeVisitor { /** The set of headers known to be part of a C++ module. Scanning can stop here. */ private Set modularHeaders; - public IncludeVisitor(Set modularHeaders) { + public LegacyIncludeVisitor(Set modularHeaders) { super( includePool, /*shutdownOnCompletion=*/ false, @@ -502,7 +528,8 @@ public IncludeVisitor(Set modularHeaders) { this.modularHeaders = modularHeaders; } - ListenableFuture processInternal( + @Override + public ListenableFuture processInternal( Artifact mainSource, Collection sources, IncludeScanningHeaderData includeScanningHeaderData, @@ -643,34 +670,42 @@ private void process( throws IOException, ExecException, InterruptedException { checkForInterrupt("processing", source); - Collection inclusions = null; + Collection inclusions; try { inclusions = - fileParseCache.computeIfAbsent( - source, - file -> { - try { - return parser.extractInclusions( - file, - actionExecutionMetadata, - actionExecutionContext, - grepIncludes, - spawnIncludeScannerSupplier.get(), - isRealOutputFile(source.getExecPath())); - } catch (IOException e) { - throw new IORuntimeException(e); - } catch (ExecException e) { - throw new ExecRuntimeException(e); - } catch (InterruptedException e) { - throw new InterruptedRuntimeException(e); - } - }); - } catch (IORuntimeException e) { - throw e.getCauseIOException(); - } catch (ExecRuntimeException e) { - throw e.getRealCause(); - } catch (InterruptedRuntimeException e) { - throw e.getRealCause(); + fileParseCache + .computeIfAbsent( + source, + file -> { + try { + return Futures.immediateFuture( + parser.extractInclusions( + file, + actionExecutionMetadata, + actionExecutionContext, + grepIncludes, + spawnIncludeScannerSupplier.get(), + isRealOutputFile(source.getExecPath()))); + } catch (IOException e) { + throw new IORuntimeException(e); + } catch (ExecException e) { + throw new ExecRuntimeException(e); + } catch (InterruptedException e) { + throw new InterruptedRuntimeException(e); + } + }) + .get(); + } catch (ExecutionException ee) { + try { + Throwables.throwIfInstanceOf(ee.getCause(), RuntimeException.class); + throw new IllegalStateException(ee.getCause()); + } catch (IORuntimeException e) { + throw e.getCauseIOException(); + } catch (ExecRuntimeException e) { + throw e.getRealCause(); + } catch (InterruptedRuntimeException e) { + throw e.getRealCause(); + } } Preconditions.checkNotNull(inclusions, source); @@ -712,7 +747,7 @@ private void processAsyncIfNotExtracted( Set visitedInclusions, final Artifact grepIncludes) throws IOException, ExecException, InterruptedException { - Collection cacheResult = fileParseCache.get(source); + ListenableFuture> cacheResult = fileParseCache.get(source); if (cacheResult != null) { process( source, @@ -910,6 +945,439 @@ private void processFileLevelHintsAsync( } } + /** + * Implements a potentially parallel traversal over source files using a thread pool shared across + * different IncludeScanner instances. + */ + private class AsyncIncludeVisitor implements IncludeVisitor { + /** The set of headers known to be part of a C++ module. Scanning can stop here. */ + private Set modularHeaders; + + public AsyncIncludeVisitor(Set modularHeaders) { + this.modularHeaders = modularHeaders; + } + + @Override + public ListenableFuture processInternal( + Artifact mainSource, + Collection sources, + IncludeScanningHeaderData includeScanningHeaderData, + List cmdlineIncludes, + Set includes, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Artifact grepIncludes, + ImmutableSet pathHints) + throws InterruptedException, IOException, ExecException { + Set visitedInclusions = Sets.newConcurrentHashSet(); + + try { + ListenableFuture result = Futures.immediateFuture(null); + // Process cmd line includes, if specified. + if (mainSource != null && !cmdlineIncludes.isEmpty()) { + result = + processCmdlineIncludesAsync( + mainSource, + cmdlineIncludes, + grepIncludes, + includes, + includeScanningHeaderData.getPathToLegalOutputArtifact(), + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions); + } + + result = + Futures.transformAsync( + result, + (v) -> + processBulkAsync( + sources, + includes, + includeScanningHeaderData.getPathToLegalOutputArtifact(), + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes), + includePool); + + // Process include hints + // TODO(ulfjack): Make this code go away. Use the new hinted inclusions instead. + Hints hints = parser.getHints(); + if (hints != null) { + // Follow "path" hints. + result = + Futures.transformAsync( + result, + (v) -> + processBulkAsync( + pathHints, + includes, + includeScanningHeaderData.getPathToLegalOutputArtifact(), + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes), + includePool); + result = + Futures.transformAsync( + result, + (v) -> + processAllFileLevelHintsAsync( + hints, + sources, + includes, + includeScanningHeaderData.getPathToLegalOutputArtifact(), + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes), + includePool); + + // Follow "file" hints for all included headers, transitively. + result = + Futures.transformAsync( + result, + (v) -> + processOnePass( + hints, + includes, + includes, + includeScanningHeaderData.getPathToLegalOutputArtifact(), + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes), + includePool); + } + return result; + } catch (IOException | InterruptedException | ExecException | MissingDepException e) { + // Careful: Do not leak visitation threads if we have an exception in the initial thread. + throw e; + } + } + + private ListenableFuture> processOnePass( + Hints hints, + Collection before, + Set includes, + Map pathToLegalOutputArtifact, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Set visitedInclusions, + Artifact grepIncludes) + throws InterruptedException, IOException, ExecException { + Set adjacent = Sets.newConcurrentHashSet(); + ListenableFuture future = + processAllFileLevelHintsAsync( + hints, + before, + adjacent, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes); + return Futures.transformAsync( + future, + (v) -> { + // Keep novel nodes as the next frontier. + for (Iterator iter = adjacent.iterator(); iter.hasNext(); ) { + if (!includes.add(iter.next())) { + iter.remove(); + } + } + if (adjacent.isEmpty()) { + return Futures.immediateFuture(null); + } + return processOnePass( + hints, + adjacent, + includes, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes); + }, + includePool); + } + + /** + * Processes a given file for includes and populates the provided set with the visited includes. + * + * @param source the file to process + * @param contextPathPos the position on the include path where the containing file was found, + * or -1 for top-level inclusions + * @param contextKind the kind how the containing file was included, or null for top-level + * inclusions + * @param visited the set to receive the files that are transitively included by {@code source} + * @param pathToLegalOutputArtifact map to look up legal output artifact by path + * @param actionExecutionMetadata owning action + * @param actionExecutionContext Services in the scope of the action, like the stream to which + * @param visitedInclusions the set of all processed inclusions, to avoid processing duplicate + * inclusions. + */ + private ListenableFuture process( + final Artifact source, + int contextPathPos, + Kind contextKind, + Set visited, + Map pathToLegalOutputArtifact, + final ActionExecutionMetadata actionExecutionMetadata, + final ActionExecutionContext actionExecutionContext, + Set visitedInclusions, + final Artifact grepIncludes) + throws IOException, InterruptedException { + checkForInterrupt("processing", source); + + ListenableFuture> actualFuture; + SettableFuture> future = SettableFuture.create(); + ListenableFuture> previous = fileParseCache.putIfAbsent(source, future); + if (previous == null) { + actualFuture = future; + future.setFuture( + parser.extractInclusionsAsync( + source, + actionExecutionMetadata, + actionExecutionContext, + grepIncludes, + spawnIncludeScannerSupplier.get(), + isRealOutputFile(source.getExecPath()))); + // When rewinding, we may need to rerun a spawn that previously failed rather than cache the + // failure here, so we remove the cache entry if the future throws. Unfortunately, we can + // only detect that case by actually calling get(). + future.addListener( + () -> { + try { + future.get(); + } catch (ExecutionException | InterruptedException e) { + fileParseCache.remove(source); + } + }, + MoreExecutors.directExecutor()); + } else { + actualFuture = previous; + } + return Futures.transformAsync( + actualFuture, + (inclusions) -> { + Preconditions.checkNotNull(inclusions, source); + // Shuffle the inclusions to get better parallelism. See b/62200470. + // Be careful not to modify the original collection! It's shared between any number of + // threads. + // TODO: Maybe we should shuffle before returning it to avoid the copy? + List shuffledInclusions = new ArrayList<>(inclusions); + Collections.shuffle(shuffledInclusions, CONSTANT_SEED_RANDOM); + + // For each inclusion: get or locate its target file & recursively process + IncludeScannerHelper helper = + new IncludeScannerHelper(includePaths, quoteIncludePaths, source); + List> allFutures = new ArrayList<>(shuffledInclusions.size()); + for (Inclusion inclusion : shuffledInclusions) { + allFutures.add( + findAndProcess( + helper.createInclusionWithContext(inclusion, contextPathPos, contextKind), + source, + visited, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes)); + } + return Futures.allAsList(allFutures); + }, + includePool); + } + + /** Visits an inclusion starting from a source file. */ + private ListenableFuture findAndProcess( + InclusionWithContext inclusion, + Artifact source, + Set visited, + Map pathToLegalOutputArtifact, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Set visitedInclusions, + Artifact grepIncludes) + throws IOException, InterruptedException { + // Try to find the included file relative to the file that contains the inclusion. Relative + // inclusions are handled like the first entry on the quote include path + Artifact includeFile = + locateRelative(inclusion.getInclusion(), pathToLegalOutputArtifact, source); + int contextPathPos = 0; + Kind contextKind = null; + + checkForInterrupt("visiting", source); + + // If nothing has been found, get an inclusion from the cache. This will automatically search + // on the include paths and populate the cache if necessary. + if (includeFile == null) { + LocateOnPathResult result = inclusionCache.lookup(inclusion, pathToLegalOutputArtifact); + includeFile = result.path; + contextPathPos = result.includePosition; + contextKind = inclusion.getContextKind(); + } + + // Recursively process the found file (if not yet done). + if (includeFile != null + && !isIllegalOutputFile(includeFile.getExecPath(), pathToLegalOutputArtifact.keySet()) + && visitedInclusions.add( + new ArtifactWithInclusionContext(includeFile, contextKind, contextPathPos))) { + visited.add(includeFile); + if (modularHeaders.contains(includeFile)) { + return Futures.immediateFuture(null); + } + return process( + includeFile, + contextPathPos, + contextKind, + visited, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes); + } + return Futures.immediateFuture(null); + } + + /** + * Processes a given list of includes for a given base file and populates the provided set with + * the visited includes + * + * @param source the source file used as a reference for finding includes + * @param includes the list of -include option strings to locate and process + * @param visited the set of files that are transitively included by {@code includes} to + * populate + * @param pathToLegalOutputArtifact map to look up legal output artifact by path + * @param actionExecutionContext Services in the scope of the action, like the stream to which + * @param visitedInclusions the set of all processed inclusions, to avoid processing duplicate + * inclusions. + */ + private ListenableFuture processCmdlineIncludesAsync( + Artifact source, + List includes, + Artifact grepIncludes, + Set visited, + Map pathToLegalOutputArtifact, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Set visitedInclusions) + throws IOException, ExecException, InterruptedException { + List> allFutures = new ArrayList<>(includes.size()); + for (String incl : includes) { + InclusionWithContext inclusion = new InclusionWithContext(incl, Kind.QUOTE); + allFutures.add( + findAndProcess( + inclusion, + source, + visited, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes)); + } + return Futures.allAsList(allFutures); + } + + /** + * Processes a bunch sources asynchronously and adds them and their included files to the + * provided set. + * + * @param sources the files to process and add to the set + * @param visited the set to receive the files that are transitively included by {@code sources} + * @param pathToLegalOutputArtifact map to look up legal output artifact by path + * @param actionExecutionContext Services in the scope of the action, like the stream to which + * @param visitedInclusions the set of all processed inclusions, to avoid processing duplicate + * inclusions. + */ + private ListenableFuture processBulkAsync( + Collection sources, + final Set visited, + final Map pathToLegalOutputArtifact, + final ActionExecutionMetadata actionExecutionMetadata, + final ActionExecutionContext actionExecutionContext, + final Set visitedInclusions, + Artifact grepIncludes) + throws IOException, InterruptedException { + List> allFutures = new ArrayList<>(sources.size()); + for (final Artifact source : sources) { + // TODO(djasper): This looks suspicious. We should only stop based on visitedInclusions. + if (!visited.add(source)) { + continue; + } + + allFutures.add( + process( + source, + /*contextPathPos=*/ -1, + /*contextKind=*/ null, + visited, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes)); + } + return Futures.allAsList(allFutures); + } + + private ListenableFuture processAllFileLevelHintsAsync( + Hints hints, + Collection sources, + Set alsoVisited, + Map pathToLegalOutputArtifact, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Set visitedInclusions, + Artifact grepIncludes) + throws InterruptedException, IOException, ExecException { + List> allFutures = new ArrayList<>(); + // Follow "file" hints for the primary sources. + for (Artifact source : sources) { + allFutures.add( + processFileLevelHintsAsync( + hints, + source, + alsoVisited, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes)); + } + return Futures.allAsList(allFutures); + } + + private ListenableFuture processFileLevelHintsAsync( + final Hints hints, + final Artifact include, + final Set alsoVisited, + final Map pathToLegalOutputArtifact, + final ActionExecutionMetadata actionExecutionMetadata, + final ActionExecutionContext actionExecutionContext, + final Set visitedInclusions, + Artifact grepIncludes) + throws InterruptedException, IOException, ExecException { + Collection sources = hints.getFileLevelHintedInclusionsLegacy(include); + // Early-out if there's nothing to do to avoid enqueuing a closure + if (sources.isEmpty()) { + return Futures.immediateFuture(null); + } + return processBulkAsync( + sources, + alsoVisited, + pathToLegalOutputArtifact, + actionExecutionMetadata, + actionExecutionContext, + visitedInclusions, + grepIncludes); + } + } + private static class ExecRuntimeException extends RuntimeException { private final ExecException cause; diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java index 8111a6628cf975..023285aa9ad90c 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java @@ -19,6 +19,10 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -36,11 +40,13 @@ import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; +import com.google.devtools.build.lib.actions.SpawnContinuation; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.includescanning.IncludeParser.GrepIncludesFileType; import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion; import com.google.devtools.build.lib.util.io.FileOutErr; +import com.google.devtools.build.lib.vfs.IORuntimeException; import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -358,12 +364,13 @@ static InputStream spawnGrep( FileOutErr originalOutErr = actionExecutionContext.getFileOutErr(); FileOutErr grepOutErr = originalOutErr.childOutErr(); SpawnActionContext context = actionExecutionContext.getContext(SpawnActionContext.class); + ActionExecutionContext spawnContext = actionExecutionContext.withFileOutErr(grepOutErr); List results; try { - results = context.exec(spawn, actionExecutionContext.withFileOutErr(grepOutErr)); - dump(actionExecutionContext, grepOutErr, originalOutErr); + results = context.exec(spawn, spawnContext); + dump(spawnContext, actionExecutionContext); } catch (ExecException e) { - dump(actionExecutionContext, grepOutErr, originalOutErr); + dump(spawnContext, actionExecutionContext); throw e; } @@ -371,10 +378,169 @@ static InputStream spawnGrep( return result.getInMemoryOutput(output); } - private static void dump(ActionExecutionContext parentContext, FileOutErr from, FileOutErr to) { - if (from.hasRecordedOutput()) { - synchronized (parentContext) { - FileOutErr.dump(from, to); + /** Extracts and returns inclusions from "file" using a spawn. */ + public ListenableFuture> extractInclusionsAsync( + Artifact file, + ActionExecutionMetadata actionExecutionMetadata, + ActionExecutionContext actionExecutionContext, + Artifact grepIncludes, + GrepIncludesFileType fileType, + boolean placeNextToFile) + throws IOException { + Path output = + getIncludesOutput( + file, actionExecutionContext.getPathResolver(), fileType, placeNextToFile); + if (!inMemoryOutput) { + AbstractAction.deleteOutput(output, placeNextToFile ? file.getRoot() : null); + if (!placeNextToFile) { + output.getParentDirectory().createDirectoryAndParents(); + } + } + + ListenableFuture dotIncludeStreamFuture = + spawnGrepAsync( + file, + execPath(output), + inMemoryOutput, + // We use {@link GrepIncludesAction} primarily to overwrite {@link Action#getMnemonic}. + // You might be tempted to use a custom mnemonic on the Spawn instead, but rest assured + // that _this does not work_. We call Spawn.getResourceOwner().getMnemonic() in a lot of + // places, some of which are downstream from here, and doing so would cause the Spawn + // and its owning ActionExecutionMetadata to be inconsistent with each other. + new GrepIncludesAction(actionExecutionMetadata, file.getExecPath()), + actionExecutionContext, + grepIncludes, + fileType); + return Futures.transform( + dotIncludeStreamFuture, + (stream) -> { + try { + return IncludeParser.processIncludes(output, stream); + } catch (IOException e) { + throw new IORuntimeException(e); + } + }, + MoreExecutors.directExecutor()); + } + + /** + * Executes grep-includes. + * + * @param input the file to parse + * @param outputExecPath the output file (exec path) + * @param inMemoryOutput if true, return the contents of the output in the return value instead of + * to the given Path + * @param resourceOwner the resource owner + * @param actionExecutionContext services in the scope of the action. Like the Err/Out stream + * outputs. + * @param fileType Either "c++" or "swig", passed verbatim to grep-includes. + * @return The InputStream of the .includes file if inMemoryOutput feature retrieved it directly. + * Otherwise "null" + * @throws ExecException if scanning fails + */ + private static ListenableFuture spawnGrepAsync( + Artifact input, + PathFragment outputExecPath, + boolean inMemoryOutput, + ActionExecutionMetadata resourceOwner, + ActionExecutionContext actionExecutionContext, + Artifact grepIncludes, + GrepIncludesFileType fileType) { + ActionInput output = ActionInputHelper.fromPath(outputExecPath); + ImmutableList inputs = ImmutableList.of(grepIncludes, input); + ImmutableList outputs = ImmutableList.of(output); + ImmutableList command = + ImmutableList.of( + grepIncludes.getExecPathString(), + input.getExecPath().getPathString(), + outputExecPath.getPathString(), + fileType.getFileType()); + + ImmutableMap.Builder execInfoBuilder = ImmutableMap.builder(); + if (inMemoryOutput) { + execInfoBuilder.put( + ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS, outputExecPath.getPathString()); + } + execInfoBuilder.put(ExecutionRequirements.DO_NOT_REPORT, ""); + + Spawn spawn = + new SimpleSpawn( + resourceOwner, + command, + ImmutableMap.of(), + execInfoBuilder.build(), + inputs, + outputs, + LOCAL_RESOURCES); + + actionExecutionContext.maybeReportSubcommand(spawn); + + // Sharing the originalOutErr across spawnGrep calls would not be thread-safe. Instead, write + // outerr to a temporary location and copy it back to the original after execution, using the + // parent context as a lock to make it thread-safe (see dump() below). + FileOutErr originalOutErr = actionExecutionContext.getFileOutErr(); + FileOutErr grepOutErr = originalOutErr.childOutErr(); + SettableFuture future = SettableFuture.create(); + ActionExecutionContext grepContext = actionExecutionContext.withFileOutErr(grepOutErr); + try { + process( + future, + SpawnContinuation.ofBeginExecution(spawn, grepContext).execute(), + output, + grepContext, + actionExecutionContext); + } catch (ExecException e) { + dump(grepContext, actionExecutionContext); + future.setException(e); + } catch (InterruptedException e) { + dump(grepContext, actionExecutionContext); + future.cancel(false); + } + return future; + } + + private static void process( + SettableFuture future, + SpawnContinuation continuation, + ActionInput output, + ActionExecutionContext actionExecutionContext, + ActionExecutionContext originalActionExecutionContext) { + if (continuation.isDone()) { + List results = continuation.get(); + dump(actionExecutionContext, originalActionExecutionContext); + SpawnResult result = Iterables.getLast(results); + InputStream stream = result.getInMemoryOutput(output); + try { + future.set( + stream == null ? actionExecutionContext.getInputPath(output).getInputStream() : stream); + } catch (IOException e) { + future.setException(e); + } + } else { + continuation + .getFuture() + .addListener( + () -> { + try { + SpawnContinuation next = continuation.execute(); + process( + future, next, output, actionExecutionContext, originalActionExecutionContext); + } catch (ExecException e) { + dump(actionExecutionContext, originalActionExecutionContext); + future.setException(e); + } catch (InterruptedException e) { + dump(actionExecutionContext, originalActionExecutionContext); + future.cancel(false); + } + }, + MoreExecutors.directExecutor()); + } + } + + private static void dump(ActionExecutionContext fromContext, ActionExecutionContext toContext) { + if (fromContext.getFileOutErr().hasRecordedOutput()) { + synchronized (toContext) { + FileOutErr.dump(fromContext.getFileOutErr(), toContext.getFileOutErr()); } } } diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/SwigIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/SwigIncludeScanner.java index f33dd026f21537..990a740048d641 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/SwigIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/SwigIncludeScanner.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.includescanning; +import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -40,11 +41,12 @@ public class SwigIncludeScanner extends LegacyIncludeScanner { public SwigIncludeScanner( ExecutorService includePool, SpawnIncludeScanner spawnIncludeScanner, - ConcurrentMap> cache, + ConcurrentMap>> cache, List includePaths, BlazeDirectories directories, ArtifactFactory artifactFactory, - Path execRoot) { + Path execRoot, + boolean useAsyncIncludeScanner) { super( new SwigIncludeParser(), includePool, @@ -55,6 +57,7 @@ public SwigIncludeScanner( directories.getOutputPath(execRoot.getBaseName()), execRoot, artifactFactory, - () -> spawnIncludeScanner); + () -> spawnIncludeScanner, + useAsyncIncludeScanner); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java index 4fc1ab47cd0359..80bbda70716a54 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java @@ -91,9 +91,9 @@ public ListenableFuture> determineAdditionalInputs( Artifact mainSource = action.getMainIncludeScannerSource(); Collection sources = action.getIncludeScannerSources(); - Profiler profiler = Profiler.instance(); try (SilentCloseable c = - profiler.profile(ProfilerTask.SCANNER, action.getSourceFile().getExecPathString())) { + Profiler.instance() + .profile(ProfilerTask.SCANNER, action.getSourceFile().getExecPathString())) { ListenableFuture future = scanner.processAsync( mainSource,