Skip to content

Commit

Permalink
Fix a source of contention in LegacyIncludeScanner.lookup(). This is …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
djasper authored and copybara-github committed Dec 8, 2020
1 parent ba5bf5f commit 364b77e
Showing 1 changed file with 10 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,16 @@ private class LegacyInclusionCache extends InclusionCache {
@Override
public LocateOnPathResult lookup(
InclusionWithContext inclusion, Map<PathFragment, Artifact> 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.
Expand Down

0 comments on commit 364b77e

Please sign in to comment.