Remove get{Analysis/Execution}Cache{Hits/Misses} methods in favor of grouping by SkyFunctions.

Filter at the callsite by class/SkyFunction instead for more precision.

PiperOrigin-RevId: 754861275
Change-Id: I7a5235745b15f8a2ee698dfb9aae22bd9d64e857

Author: jin

src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FrontierViolationChecker.java

@@ -196,8 +196,7 @@ public static RemoteAnalysisCachingDependenciesProvider check(
 
   public static void accumulateCacheHitsAcrossInvocations(
       RemoteAnalysisCachingEventListener listener) {
-    accumulatedCacheHits.getAndAdd(listener.getAnalysisNodeCacheHits());
-    accumulatedCacheHits.getAndAdd(listener.getExecutionNodeCacheHits());
+    accumulatedCacheHits.getAndAdd(listener.getCacheHits().size());
   }
 
   /**

src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingEventListener.java

@@ -29,7 +29,6 @@
 import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.Restart;
 import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.RetrievalResult;
 import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.RetrievedValue;
-import com.google.devtools.build.skyframe.ExecutionPhaseSkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import java.util.Set;
@@ -54,10 +53,6 @@ public record SerializedNodeEvent(SkyKey key) {
   private final Set<SkyKey> serializedKeys = ConcurrentHashMap.newKeySet();
   private final Set<SkyKey> cacheHits = ConcurrentHashMap.newKeySet();
   private final Set<SkyKey> cacheMisses = ConcurrentHashMap.newKeySet();
-  private final AtomicInteger analysisCacheHits = new AtomicInteger();
-  private final AtomicInteger analysisCacheMisses = new AtomicInteger();
-  private final AtomicInteger executionCacheHits = new AtomicInteger();
-  private final AtomicInteger executionCacheMisses = new AtomicInteger();
   private final Set<SerializationException> serializationExceptions = ConcurrentHashMap.newKeySet();
   private final ConcurrentHashMap<SkyFunctionName, AtomicInteger> hitsBySkyFunctionName =
       new ConcurrentHashMap<>();
@@ -103,77 +98,29 @@ public void recordRetrievalResult(RetrievalResult result, SkyKey key) {
       return;
     }
 
-    SkyFunctionName functionName = key.functionName(); // Get the function name
-
     switch (result) {
       case RetrievedValue unusedValue -> {
         if (!cacheHits.add(key)) {
           return;
         }
         hitsBySkyFunctionName
-            .computeIfAbsent(functionName, k -> new AtomicInteger())
+            .computeIfAbsent(key.functionName(), k -> new AtomicInteger())
             .incrementAndGet();
-        if (isExecutionNode(key)) {
-          executionCacheHits.incrementAndGet();
-        } else {
-          analysisCacheHits.incrementAndGet();
-        }
       }
       case NoCachedData unusedNoCachedData -> {
         if (!cacheMisses.add(key)) {
           return;
         }
         missesBySkyFunctionName
-            .computeIfAbsent(functionName, k -> new AtomicInteger())
+            .computeIfAbsent(key.functionName(), k -> new AtomicInteger())
             .incrementAndGet();
-        if (isExecutionNode(key)) {
-          executionCacheMisses.incrementAndGet();
-        } else {
-          analysisCacheMisses.incrementAndGet();
-        }
       }
       case Restart unusedRestart ->
           throw new IllegalStateException(
               "should have returned earlier"); // restart counts are not useful (yet).
     }
   }
 
-  private static boolean isExecutionNode(SkyKey key) {
-    return key instanceof ExecutionPhaseSkyKey;
-  }
-
-  /**
-   * Returns the number of successful analysis SkyValue retrievals from the {@link
-   * com.google.devtools.build.lib.skyframe.serialization.FingerprintValueService} .
-   */
-  public int getAnalysisNodeCacheHits() {
-    return analysisCacheHits.get();
-  }
-
-  /**
-   * Returns the number of successful analysis SkyValue retrievals from the {@link
-   * com.google.devtools.build.lib.skyframe.serialization.FingerprintValueService} .
-   */
-  public int getAnalysisNodeCacheMisses() {
-    return analysisCacheMisses.get();
-  }
-
-  /**
-   * Returns the number of unsuccessful execution SkyValue retrievals from the {@link
-   * com.google.devtools.build.lib.skyframe.serialization.FingerprintValueService} .
-   */
-  public int getExecutionNodeCacheHits() {
-    return executionCacheHits.get();
-  }
-
-  /**
-   * Returns the number of unsuccessful execution SkyValue retrievals from the {@link
-   * com.google.devtools.build.lib.skyframe.serialization.FingerprintValueService} .
-   */
-  public int getExecutionNodeCacheMisses() {
-    return executionCacheMisses.get();
-  }
-
   /** Returns the number of cache hits grouped by SkyFunction name. */
   public ImmutableMap<SkyFunctionName, AtomicInteger> getHitsBySkyFunctionName() {
     return ImmutableMap.copyOf(hitsBySkyFunctionName);

src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FrontierSerializerTestBase.java

@@ -441,9 +441,9 @@ public void buildCommand_downloadsFrontierBytesWithDownloadMode() throws Excepti
 
     var listener = getCommandEnvironment().getRemoteAnalysisCachingEventListener();
     // //bar:C, //bar:H, //bar:E
-    assertThat(listener.getAnalysisNodeCacheHits()).isEqualTo(3);
+    assertThat(listener.getCacheHits()).hasSize(3);
     // //bar:F is not in the project boundary, but it's in the active set, so it wasn't cached.
-    assertThat(listener.getAnalysisNodeCacheMisses()).isAtLeast(1);
+    assertThat(listener.getCacheMisses()).isNotEmpty();
   }
 
   @Test
@@ -691,20 +691,6 @@ protected void setupScenarioWithConfiguredTargets() throws Exception {
 """);
   }
 
-  protected static <T> ImmutableSet<T> filterKeys(Set<SkyKey> from, Class<? extends T> klass) {
-    return from.stream().filter(klass::isInstance).map(klass::cast).collect(toImmutableSet());
-  }
-
-  protected static ImmutableSet<Label> getLabels(Set<ActionLookupKey> from) {
-    return from.stream().map(ActionLookupKey::getLabel).collect(toImmutableSet());
-  }
-
-  protected static ImmutableSet<Label> getOwningLabels(Set<ActionLookupData> from) {
-    return from.stream()
-        .map(data -> data.getActionLookupKey().getLabel())
-        .collect(toImmutableSet());
-  }
-
   @Test
   public void actionLookupKey_underTheFrontier_areNotUploaded() throws Exception {
     setupGenruleGraph();
@@ -1254,4 +1240,18 @@ public void workspaceInit(
       builder.setSyscallCache(syscallCache);
     }
   }
+
+  protected static <T> ImmutableSet<T> filterKeys(Set<SkyKey> from, Class<? extends T> klass) {
+    return from.stream().filter(klass::isInstance).map(klass::cast).collect(toImmutableSet());
+  }
+
+  protected static ImmutableSet<Label> getLabels(Set<ActionLookupKey> from) {
+    return from.stream().map(ActionLookupKey::getLabel).collect(toImmutableSet());
+  }
+
+  protected static ImmutableSet<Label> getOwningLabels(Set<ActionLookupData> from) {
+    return from.stream()
+        .map(data -> data.getActionLookupKey().getLabel())
+        .collect(toImmutableSet());
+  }
 }