From 2388856cb3a763793bbc1d1060fc82eb589ae1f7 Mon Sep 17 00:00:00 2001 From: Tiago Quelhas Date: Thu, 18 Jan 2024 18:12:41 +0100 Subject: [PATCH] Distinguish the remote, disk and combined caches in the UI state tracker. --- .../build/lib/remote/GrpcCacheClient.java | 5 +++++ .../devtools/build/lib/remote/RemoteCache.java | 4 ++++ .../build/lib/remote/RemoteExecutionService.java | 9 +++++++++ .../build/lib/remote/RemoteSpawnCache.java | 15 ++++++++++++++- .../lib/remote/common/RemoteCacheClient.java | 3 +++ .../lib/remote/disk/DiskAndRemoteCacheClient.java | 5 +++++ .../build/lib/remote/disk/DiskCacheClient.java | 5 +++++ .../build/lib/remote/http/HttpCacheClient.java | 5 +++++ .../build/lib/remote/RemoteSpawnCacheTest.java | 2 ++ .../lib/remote/util/InMemoryCacheClient.java | 5 +++++ 10 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java index 6d142bf5df60cb..41b866b034aa10 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java @@ -117,6 +117,11 @@ public GrpcCacheClient( maxMissingBlobsDigestsPerMessage > 0, "Error: gRPC message size too small."); } + @Override + public String getDisplayName() { + return "remote-cache"; + } + private int computeMaxMissingBlobsDigestsPerMessage() { final int overhead = FindMissingBlobsRequest.newBuilder() diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index 36f97298bb6260..60892f4376c081 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -95,6 +95,10 @@ public RemoteCache( this.options = options; this.digestUtil = digestUtil; } + + public String getDisplayName() { + return cacheProtocol.getDisplayName(); + } public CacheCapabilities getCacheCapabilities() throws IOException { return cacheProtocol.getCacheCapabilities(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index e6890842321789..9545b39328344c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -289,6 +289,15 @@ private static boolean useDiskCache(RemoteOptions options) { return options.diskCache != null && !options.diskCache.isEmpty(); } + /** + * If using a disk, remote or combined cache, returns the display name for the cache. Otherwise, + * returns null. + */ + @Nullable + public String getCacheDisplayName() { + return remoteCache != null ? remoteCache.getDisplayName() : null; + } + public CachePolicy getReadCachePolicy(Spawn spawn) { if (remoteCache == null) { return CachePolicy.NO_CACHE; diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 0ad170332e5cb8..53e308130e56db 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.remote; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.devtools.build.lib.profiler.ProfilerTask.REMOTE_DOWNLOAD; import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult; @@ -46,6 +47,7 @@ import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.util.NoSuchElementException; +import java.util.concurrent.ConcurrentHashMap; /** A remote {@link SpawnCache} implementation. */ @ThreadSafe // If the RemoteActionCache implementation is thread-safe. @@ -60,6 +62,10 @@ final class RemoteSpawnCache implements SpawnCache { private final DigestUtil digestUtil; private final boolean verboseFailures; + // Maps cache display names to lazily-initialized, reusable singleton events, to reduce garbage. + private static final ConcurrentHashMap EVENT_MAP = + new ConcurrentHashMap<>(); + RemoteSpawnCache( Path execRoot, RemoteOptions options, @@ -101,7 +107,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) Profiler prof = Profiler.instance(); if (shouldAcceptCachedResult) { - context.report(SPAWN_CHECKING_CACHE_EVENT); + reportSpawnCheckingCacheEvent(context); // Metadata will be available in context.current() until we detach. // This is done via a thread-local variable. try { @@ -212,6 +218,13 @@ private void checkForConcurrentModifications() } } + private void reportSpawnCheckingCacheEvent(SpawnExecutionContext context) { + String displayName = checkNotNull(remoteExecutionService.getCacheDisplayName()); + SpawnCheckingCacheEvent event = EVENT_MAP.computeIfAbsent(displayName, + SpawnCheckingCacheEvent::create); + context.report(event); + } + @Override public boolean usefulInDynamicExecution() { return false; diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java index 9e0db4e5c053d7..c9bfcab9ac0016 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java @@ -33,6 +33,9 @@ *

Implementations must be thread-safe. */ public interface RemoteCacheClient extends MissingDigestsFinder { + + String getDisplayName(); + CacheCapabilities getCacheCapabilities() throws IOException; ListenableFuture getAuthority(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java index 67e71faf826c9c..2b519d858424bd 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java @@ -49,6 +49,11 @@ public DiskAndRemoteCacheClient(DiskCacheClient diskCache, RemoteCacheClient rem this.remoteCache = Preconditions.checkNotNull(remoteCache); } + @Override + public String getDisplayName() { + return "combined-cache"; + } + @Override public ListenableFuture uploadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, ActionResult actionResult) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java index dd3de658bb8ce9..71e8b4da78573d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java @@ -84,6 +84,11 @@ public DiskCacheClient(Path root, boolean verifyDownloads, DigestUtil digestUtil this.root.createDirectoryAndParents(); } + @Override + public String getDisplayName() { + return "disk-cache"; + } + /** * If the given path exists, updates its mtime and returns true. Otherwise, returns false. * diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java index 608eae019e3d7e..6871ead3e5ce8e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java @@ -300,6 +300,11 @@ public void channelCreated(Channel ch) { this.retrier = retrier; } + @Override + public String getDisplayName() { + return "remote-cache"; + } + @SuppressWarnings("FutureReturnValueIgnored") private Promise acquireUploadChannel() { Promise channelReady = eventLoop.next().newPromise(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index e1d8d2187162bc..f839f033d8fbef 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -288,6 +288,8 @@ public final void setUp() throws Exception { remotePathResolver = RemotePathResolver.createDefault(execRoot); fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().getSingleton(), "xyz"); + + when(remoteCache.getDisplayName()).thenReturn("remote-cache"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java index dee425d789b0a7..a3b2252e58c2b0 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java @@ -59,6 +59,11 @@ public InMemoryCacheClient(Map casEntries) { } } + @Override + public String getDisplayName() { + return "in-memory-cache"; + } + public InMemoryCacheClient() { this.cas = new ConcurrentHashMap<>(); }