Disable pooled SkyKeyInterner for PackageLoader

Pooled `SkyKeyInterner` assumes that there is only one pool existing in the blaze program, so setting the pool to some container or `null` happens sequentially without synchronization concern. However, this assumption is broken if multiple `PackageLoader` instances are being used with concurrent Skyframe evaluations for other programs than blaze.

It is possible that one `PackageLoader` tries to access some SkyKey in the globalPool, but the pool has already been reset by the other `PackageLoader`. In order to avoid this, we disable pooled `SkyKeyInterner` for `PackageLoader` and `SkyKey`s will revert to use the naive weak interner.

PiperOrigin-RevId: 531318564
Change-Id: Ie2c7793c3fdc1ad3b6677c13f0d8c00ca8f4125e

Author: yuyue730

src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java

@@ -140,7 +140,6 @@ public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVers
   private final int nonSkyframeGlobbingThreads;
   @VisibleForTesting final ForkJoinPool forkJoinPoolForNonSkyframeGlobbing;
   private final int skyframeThreads;
-  private final boolean usePooledInterning;
 
   /** Abstract base class of a builder for {@link PackageLoader} instances. */
   public abstract static class Builder {
@@ -157,7 +156,6 @@ public abstract static class Builder {
     List<PrecomputedValue.Injected> extraPrecomputedValues = new ArrayList<>();
     int nonSkyframeGlobbingThreads = 1;
     int skyframeThreads = 1;
-    boolean usePooledInterning = true;
 
     protected Builder(
         Root workspaceDir,
@@ -243,12 +241,6 @@ public Builder setExternalFileAction(ExternalFileAction externalFileAction) {
       return this;
     }
 
-    @CanIgnoreReturnValue
-    public Builder disablePooledSkyKeyInterning() {
-      this.usePooledInterning = false;
-      return this;
-    }
-
     /** Throws {@link IllegalArgumentException} if builder args are incomplete/inconsistent. */
     protected void validate() {
       if (starlarkSemantics == null) {
@@ -280,7 +272,6 @@ public final PackageLoader build() {
         NamedForkJoinPool.newNamedPool(
             "package-loader-globbing-pool", builder.nonSkyframeGlobbingThreads);
     this.skyframeThreads = builder.skyframeThreads;
-    this.usePooledInterning = builder.usePooledInterning;
     this.directories = builder.directories;
     this.hashFunction = builder.workspaceDir.getFileSystem().getDigestFunction().getHashFunction();
 
@@ -362,14 +353,8 @@ private Result loadPackagesInternal(
       StoredEventHandler storedEventHandler)
       throws InterruptedException {
     MemoizingEvaluator evaluator = makeFreshEvaluator();
-    EvaluationResult<PackageValue> evalResult;
-    try {
-      evalResult = evaluator.evaluate(pkgKeys, evaluationContext);
-    } finally {
-      if (usePooledInterning) {
-        evaluator.cleanupInterningPools();
-      }
-    }
+    EvaluationResult<PackageValue> evalResult = evaluator.evaluate(pkgKeys, evaluationContext);
+
     ImmutableMap.Builder<PackageIdentifier, PackageLoader.PackageOrException> result =
         ImmutableMap.builder();
     for (SkyKey key : pkgKeys) {
@@ -417,7 +402,9 @@ private MemoizingEvaluator makeFreshEvaluator() {
         EventFilter.FULL_STORAGE,
         new EmittedEventState(),
         /* keepEdges= */ false,
-        usePooledInterning);
+        // Using pooled interner is unsound if there are multiple MemoizingEvaluators evaluating
+        // concurrently.
+        /* usePooledInterning= */ false);
   }
 
   protected abstract CrossRepositoryLabelViolationStrategy