Fix FrontierNodeVersion multiple instantiation and add logging.

getSkyValueVersion calls were still creating multiple FrontierNodeVersion
objects, and I realized that it was missing double-checked locking in the
synchronized block.

Also remove the call to serialize a simple string repr of the active directories -- we can pass the actual byte array instead.

Also added a log line for the components of the skyvalue version for debugging.

```
241024 09:20:21.005:I 51 [com.google.devtools.build.lib.buildtool.BuildTool.buildTargets:223] Build identifier: 2d2c2b5c-1930-4d5f-86a2-759b3e3814f7
241024 09:21:26.650:I 2486 [com.google.devtools.build.lib.buildtool.BuildTool$RemoteAnalysisCachingDependenciesProviderImpl.getSkyValueVersion:1182] Remote analysis caching SkyValue version: FrontierNodeVersion{topLevelConfig=-75950329, directoryMatcher=-1987693526, blazeInstall=-542573734, precomputed=688156809}
```

PiperOrigin-RevId: 689369335
Change-Id: I0931bc6662d83b5e5a107aa541715b509be67857

src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java

@@ -1157,17 +1157,31 @@ public boolean withinActiveDirectories(PackageIdentifier pkg) {
       return activeDirectoriesMatcher.includes(pkg.getPackageFragment());
     }
 
-    private FrontierNodeVersion frontierNodeVersionSingleton = null;
-
+    private volatile FrontierNodeVersion frontierNodeVersionSingleton = null;
+
+    /**
+     * Returns a byte array to uniquely version SkyValues for serialization.
+     *
+     * <p>This should only be caalled when Bazel has determined values for all version components
+     * for instantiating a {@link FrontierNodeVersion}.
+     *
+     * <p>This could be in the constructor if we know about the {@code topLevelConfig} component at
+     * initialization, but it is created much later during the deserialization pass.
+     */
     @Override
-    public FrontierNodeVersion getSkyValueVersion() throws SerializationException {
+    public FrontierNodeVersion getSkyValueVersion() {
       if (frontierNodeVersionSingleton == null) {
         synchronized (this) {
-          frontierNodeVersionSingleton =
-              new FrontierNodeVersion(
-                  topLevelConfig.checksum(),
-                  getObjectCodecs().serializeMemoized(activeDirectoriesMatcher.toString()),
-                  blazeInstallMD5);
+          // Avoid re-initializing the value for subsequent threads with double checked locking.
+          if (frontierNodeVersionSingleton == null) {
+            frontierNodeVersionSingleton =
+                new FrontierNodeVersion(
+                    topLevelConfig.checksum(),
+                    activeDirectoriesMatcher.toString(),
+                    blazeInstallMD5);
+            logger.atInfo().log(
+                "Remote analysis caching SkyValue version: %s", frontierNodeVersionSingleton);
+          }
         }
       }
       return frontierNodeVersionSingleton;

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

@@ -20,6 +20,7 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.hash.HashCode;
 import com.google.common.primitives.Bytes;
 import com.google.common.util.concurrent.Futures;
@@ -328,20 +329,19 @@ private SkyValueRetriever() {}
   /** A tuple representing the version of a cached SkyValue in the frontier. */
   public static final class FrontierNodeVersion {
     public static final FrontierNodeVersion CONSTANT_FOR_TESTING =
-        new FrontierNodeVersion(
-            "123", ByteString.copyFrom(new byte[] {1, 2, 3}), HashCode.fromInt(42));
+        new FrontierNodeVersion("123", "string_for_testing", HashCode.fromInt(42));
     private final byte[] topLevelConfigFingerprint;
     private final byte[] directoryMatcherFingerprint;
     private final byte[] blazeInstallMD5Fingerprint;
     private final byte[] precomputedFingerprint;
 
     public FrontierNodeVersion(
         String topLevelConfigChecksum,
-        ByteString directoryMatcherFingerprint,
+        String directoryMatcherStringRepr,
         HashCode blazeInstallMD5) {
       // TODO: b/364831651 - add more fields like source and blaze versions.
       this.topLevelConfigFingerprint = topLevelConfigChecksum.getBytes(UTF_8);
-      this.directoryMatcherFingerprint = directoryMatcherFingerprint.toByteArray();
+      this.directoryMatcherFingerprint = directoryMatcherStringRepr.getBytes(UTF_8);
       this.blazeInstallMD5Fingerprint = blazeInstallMD5.asBytes();
       this.precomputedFingerprint =
           Bytes.concat(
@@ -358,6 +358,16 @@ public byte[] concat(byte[] input) {
       return Bytes.concat(precomputedFingerprint, input);
     }
 
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)
+          .add("topLevelConfig", Arrays.hashCode(topLevelConfigFingerprint))
+          .add("directoryMatcher", Arrays.hashCode(directoryMatcherFingerprint))
+          .add("blazeInstall", Arrays.hashCode(blazeInstallMD5Fingerprint))
+          .add("precomputed", hashCode())
+          .toString();
+    }
+
     @Override
     public int hashCode() {
       return Arrays.hashCode(precomputedFingerprint);

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

@@ -167,7 +167,7 @@ public void waitingForFutureValueBytes_withMatchingDistinguisher_returnsImmediat
     var version =
         new FrontierNodeVersion(
             /* topLevelConfigChecksum= */ "42",
-            /* directoryMatcherFingerprint= */ ByteString.copyFrom(new byte[] {1, 2, 3}),
+            /* directoryMatcherStringRepr= */ "some_string",
             /* blazeInstallMD5= */ HashCode.fromInt(42));
     uploadKeyValuePair(key, version, value, fingerprintValueService);
 
@@ -194,7 +194,7 @@ public void waitingForFutureValueBytes_withNonMatchingDistinguisher_returnsNoCac
     var version =
         new FrontierNodeVersion(
             /* topLevelConfigChecksum= */ "42",
-            /* directoryMatcherFingerprint= */ ByteString.copyFrom(new byte[] {1, 2, 3}),
+            /* directoryMatcherStringRepr= */ "some_string",
             /* blazeInstallMD5= */ HashCode.fromInt(42));
     uploadKeyValuePair(key, version, value, fingerprintValueService);
 
@@ -208,7 +208,7 @@ public void waitingForFutureValueBytes_withNonMatchingDistinguisher_returnsNoCac
             /* stateProvider= */ this,
             /* frontierNodeVersion= */ new FrontierNodeVersion(
                 /* topLevelConfigChecksum= */ "9000",
-                /* directoryMatcherFingerprint= */ ByteString.copyFrom(new byte[] {7, 8, 9}),
+                /* directoryMatcherStringRepr= */ "another_string",
                 /* blazeInstallMD5= */ HashCode.fromInt(9000)));
 
     assertThat(result).isSameInstanceAs(NO_CACHED_DATA);
@@ -546,43 +546,35 @@ public void exceptionWhileWaitingForResult_throwsException() throws Exception {
 
   @Test
   public void frontierNodeVersions_areEqual_ifTupleComponentsAreEqual() {
-    var first =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("bar"), HashCode.fromInt(42));
-    var second =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("bar"), HashCode.fromInt(42));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
+    var second = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
 
     assertThat(first.getPrecomputedFingerprint()).isEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isEqualTo(second);
   }
 
   @Test
   public void frontierNodeVersions_areNotEqual_ifTopLevelConfigChecksumIsDifferent() {
-    var first =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("bar"), HashCode.fromInt(42));
-    var second =
-        new FrontierNodeVersion("CHANGED", ByteString.copyFromUtf8("bar"), HashCode.fromInt(42));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
+    var second = new FrontierNodeVersion("CHANGED", "bar", HashCode.fromInt(42));
 
     assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isNotEqualTo(second);
   }
 
   @Test
   public void frontierNodeVersions_areNotEqual_ifActiveDirectoriesAreDifferent() {
-    var first =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("bar"), HashCode.fromInt(42));
-    var second =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("CHANGED"), HashCode.fromInt(42));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
+    var second = new FrontierNodeVersion("foo", "CHANGED", HashCode.fromInt(42));
 
     assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isNotEqualTo(second);
   }
 
   @Test
   public void frontierNodeVersions_areNotEqual_ifBlazeInstallMD5IsDifferent() {
-    var first =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("bar"), HashCode.fromInt(42));
-    var second =
-        new FrontierNodeVersion("foo", ByteString.copyFromUtf8("bar"), HashCode.fromInt(9000));
+    var first = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(42));
+    var second = new FrontierNodeVersion("foo", "bar", HashCode.fromInt(9000));
 
     assertThat(first.getPrecomputedFingerprint()).isNotEqualTo(second.getPrecomputedFingerprint());
     assertThat(first).isNotEqualTo(second);