Add functionality to make certain SkyValues unshareable, meaning they are not serialized. Tag TestCompletionValue and any ActionExecutionValue coming from a NotifyOnActionCacheHit (i.e., tests) like that. To make such values really not shared, request the ActionExecutionValue from TestCompletionFunction as opposed to the ArtifactValue (propagating the unshareable bit up seemed like too much fuss, and I have a dream of getting rid of ArtifactValue anyway).

PiperOrigin-RevId: 200504358

3 files changed, 87 insertions, 35 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD
index 34ad3a7efc..8a57fd0d35 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/skyframe/BUILD
@@ -5,10 +5,11 @@ package(
 )
 
 SKYFRAME_OBJECT_SRCS = [
-    "SkyValue.java",
-    "SkyKey.java",
     "AbstractSkyKey.java",
+    "ShareabilityOfValue.java",
     "SkyFunctionName.java",
+    "SkyKey.java",
+    "SkyValue.java",
 ]
 
 java_library(
@@ -16,7 +17,6 @@ java_library(
     srcs = SKYFRAME_OBJECT_SRCS,
     visibility = ["//visibility:public"],
     deps = [
-        "//src/main/java/com/google/devtools/build/lib/collect",
         "//third_party:guava",
     ],
 )
diff --git a/src/main/java/com/google/devtools/build/skyframe/ShareabilityOfValue.java b/src/main/java/com/google/devtools/build/skyframe/ShareabilityOfValue.java
new file mode 100644
index 0000000000..d9f9899b9e
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/skyframe/ShareabilityOfValue.java
@@ -0,0 +1,32 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.skyframe;
+
+/**
+ * When the {@link NodeEntry#getValue} corresponding to a given {@link SkyFunctionName} is
+ * shareable: always, sometimes (depending on the specific key argument and/or value), or never.
+ *
+ * <p>Values may be unshareable because they are just not serializable, or because they contain data
+ * that cannot safely be re-used as-is by another invocation.
+ *
+ * <p>Unshareable data should not be serialized, since it will never be re-used. Attempts to fetch
+ * serialized data will check this value and only perform the fetch if the value is not {@link
+ * #NEVER}.
+ */
+public enum ShareabilityOfValue {
+  ALWAYS,
+  SOMETIMES,
+  NEVER
+}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
index af7474c506..3d6ece1975 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
@@ -13,30 +13,18 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
+import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
+import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-
 import java.io.Serializable;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 
 /** An identifier for a {@code SkyFunction}. */
 public final class SkyFunctionName implements Serializable {
-
-  // In practice the number of unique SkyFunctionNames should be reasonably limited, use this cache
-  // to avoid accidentally creating many of the same.
-  private static final LoadingCache<String, SkyFunctionName> skyFunctionNameCache =
-      CacheBuilder.newBuilder()
-        .weakValues()
-        .build(
-            new CacheLoader<String, SkyFunctionName>() {
-              @Override
-              public SkyFunctionName load(String name) {
-                return new SkyFunctionName(name);
-              }
-            });
+  private static final Cache<NameOnlyWrapper, SkyFunctionName> interner =
+      CacheBuilder.newBuilder().build();
 
   /**
    * A well-known key type intended for testing only. The associated SkyKey should have a String
@@ -47,26 +35,45 @@ public final class SkyFunctionName implements Serializable {
 
   /** Create a SkyFunctionName identified by {@code name}. */
   public static SkyFunctionName create(String name) {
+    return create(name, ShareabilityOfValue.ALWAYS);
+  }
+
+  public static SkyFunctionName create(String name, ShareabilityOfValue shareabilityOfValue) {
+    SkyFunctionName result = new SkyFunctionName(name, shareabilityOfValue);
+    SkyFunctionName cached;
     try {
-      return skyFunctionNameCache.get(name);
+      cached = interner.get(new NameOnlyWrapper(result), () -> result);
     } catch (ExecutionException e) {
-      throw new IllegalStateException("Unexpected exception creating SkyFunctionName", e);
+      throw new IllegalStateException(e);
     }
+    Preconditions.checkState(
+        result.equals(cached),
+        "Tried to create SkyFunctionName objects with same name but different properties: %s %s",
+        result,
+        cached);
+    return cached;
   }
 
   private final String name;
+  private final ShareabilityOfValue shareabilityOfValue;
 
-  private SkyFunctionName(String name) {
+  private SkyFunctionName(String name, ShareabilityOfValue shareabilityOfValue) {
     this.name = name;
+    this.shareabilityOfValue = shareabilityOfValue;
   }
 
   public String getName() {
     return name;
   }
 
+  public ShareabilityOfValue getShareabilityOfValue() {
+    return shareabilityOfValue;
+  }
+
   @Override
   public String toString() {
-    return name;
+    return name
+        + (shareabilityOfValue.equals(ShareabilityOfValue.ALWAYS) ? "" : " " + shareabilityOfValue);
   }
 
   @Override
@@ -78,11 +85,12 @@ public final class SkyFunctionName implements Serializable {
       return false;
     }
     SkyFunctionName other = (SkyFunctionName) obj;
-    return name.equals(other.name);
+    return name.equals(other.name) && shareabilityOfValue.equals(other.shareabilityOfValue);
   }
 
   @Override
   public int hashCode() {
+    // Don't bother incorporating serializabilityOfValue into hashCode: should always be the same.
     return name.hashCode();
   }
 
@@ -90,23 +98,35 @@ public final class SkyFunctionName implements Serializable {
    * A predicate that returns true for {@link SkyKey}s that have the given {@link SkyFunctionName}.
    */
   public static Predicate<SkyKey> functionIs(final SkyFunctionName functionName) {
-    return new Predicate<SkyKey>() {
-      @Override
-      public boolean apply(SkyKey skyKey) {
-        return functionName.equals(skyKey.functionName());
-      }
-    };
+    return skyKey -> functionName.equals(skyKey.functionName());
   }
 
   /**
    * A predicate that returns true for {@link SkyKey}s that have the given {@link SkyFunctionName}.
    */
   public static Predicate<SkyKey> functionIsIn(final Set<SkyFunctionName> functionNames) {
-    return new Predicate<SkyKey>() {
-      @Override
-      public boolean apply(SkyKey skyKey) {
-        return functionNames.contains(skyKey.functionName());
+    return skyKey -> functionNames.contains(skyKey.functionName());
+  }
+
+  private static class NameOnlyWrapper {
+    private final SkyFunctionName skyFunctionName;
+
+    private NameOnlyWrapper(SkyFunctionName skyFunctionName) {
+      this.skyFunctionName = skyFunctionName;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof NameOnlyWrapper)) {
+        return false;
       }
-    };
+      SkyFunctionName thatFunctionName = ((NameOnlyWrapper) obj).skyFunctionName;
+      return this.skyFunctionName.getName().equals(thatFunctionName.name);
+    }
+
+    @Override
+    public int hashCode() {
+      return skyFunctionName.getName().hashCode();
+    }
   }
 }