From 2aa73ab1e0b50c65c3f1fb2fd7c0fc043f5350dd Mon Sep 17 00:00:00 2001 From: ulfjack Date: Wed, 8 May 2019 08:55:27 -0700 Subject: [PATCH] Refactor process to continue directly if possible After this change, the async include scanner processes discovered inclusions in the same thread if it doesn't have to wait for them. This seems to have a small positive impact on build times. PiperOrigin-RevId: 247221888 --- .../includescanning/LegacyIncludeScanner.java | 70 ++++++++++++------- 1 file changed, 45 insertions(+), 25 deletions(-) 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 1d47588a090581..ea267349547bc6 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 @@ -946,31 +946,51 @@ private ListenableFuture process( } 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)); - } - return Futures.allAsList(allFutures); - }, - includePool); + + if (actualFuture.isDone()) { + Collection inclusions; + try { + inclusions = actualFuture.get(); + } catch (ExecutionException e) { + return actualFuture; + } + return processInclusions(inclusions, source, contextPathPos, contextKind, visited); + } else { + return Futures.transformAsync( + actualFuture, + (inclusions) -> + processInclusions(inclusions, source, contextPathPos, contextKind, visited), + includePool); + } + } + + private ListenableFuture processInclusions( + Collection inclusions, + Artifact source, + int contextPathPos, + Kind contextKind, + Set visited) + throws IOException, InterruptedException { + 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)); + } + return Futures.allAsList(allFutures); } /** Visits an inclusion starting from a source file. */