Skip to content

Commit

Permalink
Distinguish the remote, disk and combined caches in the UI state trac…
Browse files Browse the repository at this point in the history
…ker.
  • Loading branch information
tjgq committed Jan 18, 2024
1 parent 084dadf commit 2388856
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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<String, SpawnCheckingCacheEvent> EVENT_MAP =
new ConcurrentHashMap<>();

RemoteSpawnCache(
Path execRoot,
RemoteOptions options,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
* <p>Implementations must be thread-safe.
*/
public interface RemoteCacheClient extends MissingDigestsFinder {

String getDisplayName();

CacheCapabilities getCacheCapabilities() throws IOException;

ListenableFuture<String> getAuthority();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Void> uploadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, ActionResult actionResult) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ public void channelCreated(Channel ch) {
this.retrier = retrier;
}

@Override
public String getDisplayName() {
return "remote-cache";
}

@SuppressWarnings("FutureReturnValueIgnored")
private Promise<Channel> acquireUploadChannel() {
Promise<Channel> channelReady = eventLoop.next().newPromise();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public InMemoryCacheClient(Map<Digest, byte[]> casEntries) {
}
}

@Override
public String getDisplayName() {
return "in-memory-cache";
}

public InMemoryCacheClient() {
this.cas = new ConcurrentHashMap<>();
}
Expand Down

0 comments on commit 2388856

Please sign in to comment.