From 364b77ed34afcd07d48313472e8fe3b260a8312f Mon Sep 17 00:00:00 2001 From: djasper Date: Tue, 8 Dec 2020 01:50:06 -0800 Subject: [PATCH] Fix a source of contention in LegacyIncludeScanner.lookup(). This is another instance where CompactHashMap's computeIfAbsent() is used with a computation that is not short and simple, leading to contention. RELNOTES: None. PiperOrigin-RevId: 346276142 --- .../lib/includescanning/LegacyIncludeScanner.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 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 9a689db2c06633..c6239dd157fe84 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 @@ -353,8 +353,16 @@ private class LegacyInclusionCache extends InclusionCache { @Override public LocateOnPathResult lookup( InclusionWithContext inclusion, Map pathToDeclaredHeader) { - LocateOnPathResult result = - cache.computeIfAbsent(inclusion, key -> locateOnPaths(key, pathToDeclaredHeader, false)); + LocateOnPathResult result = cache.get(inclusion); + if (result == null) { + // Do not use computeIfAbsent() as the implementation of locateOnPaths might do multiple + // file stats and this creates substantial contention given CompactHashMap's locking. + // Do not use futures as the few duplicate executions are cheaper than the additional memory + // that would be required. + result = locateOnPaths(inclusion, pathToDeclaredHeader, false); + cache.put(inclusion, result); + return result; + } // If the previous computation for this inclusion had a different pathToDeclaredHeader // map, result may not be valid for this lookup. Because this is a hot spot, we tolerate a // known correctness bug but try to catch most issues.