Reduce needless contention in the include scanner's fileParseCache. The
computeIfAbsent() function of ConcurrentHashMap is supposed to have a fast
implementation and the map will then allow ~number of processors concurrent
writes. However, reading a file and extracting the include lines can be a costly
IO operation that does not fit this bill. Use futures instead.
RELNOTES: None.
PiperOrigin-RevId: 344799253
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 5dd4f42..9a689db 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,7 +15,6 @@
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;
@@ -58,6 +57,7 @@
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
/**
* C include scanner. Quickly scans C/C++ source files to determine the bounding set of transitively
@@ -858,42 +858,37 @@
checkForInterrupt("processing", source);
Collection<Inclusion> inclusions;
- try {
- inclusions =
- 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) {
+ SettableFuture<Collection<Inclusion>> future = SettableFuture.create();
+ Future<Collection<Inclusion>> previous = fileParseCache.putIfAbsent(source, future);
+ if (previous == null) {
+ previous = future;
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();
+ future.set(
+ parser.extractInclusions(
+ source,
+ actionExecutionMetadata,
+ actionExecutionContext,
+ grepIncludes,
+ spawnIncludeScannerSupplier.get(),
+ isRealOutputFile(source.getExecPath())));
+ } catch (IOException | ExecException | InterruptedException e) {
+ future.setException(e);
+ fileParseCache.remove(source);
+ throw e;
}
}
+ try {
+ inclusions = previous.get();
+ } catch (ExecutionException e) {
+ if (e.getCause() instanceof InterruptedException) {
+ throw (InterruptedException) e.getCause();
+ } else if (e.getCause() instanceof ExecException) {
+ throw (ExecException) e.getCause();
+ } else if (e.getCause() instanceof IOException) {
+ throw (IOException) e.getCause();
+ }
+ throw new IllegalStateException(e.getCause());
+ }
Preconditions.checkNotNull(inclusions, source);
// Shuffle the inclusions to get better parallelism. See b/62200470.
