1 files changed, 50 insertions, 28 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
index 64df5c166b..cbe30564be 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SingleBuildFileCache.java
@@ -24,7 +24,6 @@ import com.google.common.io.BaseEncoding;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
 import com.google.devtools.build.lib.actions.DigestOfDirectoryException;
-import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.protobuf.ByteString;
@@ -56,7 +55,7 @@ public class SingleBuildFileCache implements ActionInputFileCache {
   // If we can't get the digest, we store the exception. This avoids extra file IO for files
   // that are allowed to be missing, as we first check a likely non-existent content file
   // first.  Further we won't need to unwrap the exception in getDigest().
-  private final LoadingCache<ActionInput, Pair<ByteString, IOException>> pathToDigest =
+  private final LoadingCache<ActionInput, ActionInputMetadata> pathToMetadata =
       CacheBuilder.newBuilder()
       // We default to 10 disk read threads, but we don't expect them all to edit the map
       // simultaneously.
@@ -64,9 +63,9 @@ public class SingleBuildFileCache implements ActionInputFileCache {
       // Even small-ish builds, as of 11/21/2011 typically have over 10k artifacts, so it's
       // unlikely that this default will adversely affect memory in most cases.
       .initialCapacity(10000)
-      .build(new CacheLoader<ActionInput, Pair<ByteString, IOException>>() {
+      .build(new CacheLoader<ActionInput, ActionInputMetadata>() {
         @Override
-        public Pair<ByteString, IOException> load(ActionInput input) {
+        public ActionInputMetadata load(ActionInput input) {
           Path path = null;
           try {
             path = fs.getPath(fullPath(input));
@@ -74,29 +73,25 @@ public class SingleBuildFileCache implements ActionInputFileCache {
             ByteString digest = ByteString.copyFrom(
                 hex.encode(path.getMD5Digest())
                    .getBytes(US_ASCII));
-            pathToBytes.put(input, path.getFileSize());
             // Inject reverse mapping. Doing this unconditionally in getDigest() showed up
             // as a hotspot in CPU profiling.
             digestToPath.put(digest, input);
-            return Pair.of(digest, null);
+            return new ActionInputMetadata(digest, path.getFileSize());
           } catch (IOException e) {
             if (path != null && path.isDirectory()) {
-              pathToBytes.put(input, 0L);
-              return Pair.<ByteString, IOException>of(null, new DigestOfDirectoryException(
+              // TODO(bazel-team): This is rather presumptuous- it could have been another type of
+              // IOException.
+              return new ActionInputMetadata(new DigestOfDirectoryException(
                   "Input is a directory: " + input.getExecPathString()));
+            } else {
+              return new ActionInputMetadata(e);
             }
-
-            // Put value into size map to avoid trying to read file again later.
-            pathToBytes.put(input, 0L);
-            return Pair.of(null, e);
           }
         }
       });
 
   private final Map<ByteString, ActionInput> digestToPath = Maps.newConcurrentMap();
 
-  private final Map<ActionInput, Long> pathToBytes = Maps.newConcurrentMap();
-
   @Nullable
   @Override
   public ActionInput getInputFromDigest(ByteString digest) {
@@ -110,24 +105,12 @@ public class SingleBuildFileCache implements ActionInputFileCache {
 
   @Override
   public long getSizeInBytes(ActionInput input) throws IOException {
-    // TODO(bazel-team): this only works if pathToDigest has already been called.
-    Long sz = pathToBytes.get(input);
-    if (sz != null) {
-      return sz;
-    }
-    Path path = fs.getPath(fullPath(input));
-    sz = path.getFileSize();
-    pathToBytes.put(input, sz);
-    return sz;
+    return pathToMetadata.getUnchecked(input).getSize();
   }
 
   @Override
   public ByteString getDigest(ActionInput input) throws IOException {
-    Pair<ByteString, IOException> result = pathToDigest.getUnchecked(input);
-    if (result.second != null) {
-      throw result.second;
-    }
-    return result.first;
+    return pathToMetadata.getUnchecked(input).getDigest();
   }
 
   @Override
@@ -144,4 +127,43 @@ public class SingleBuildFileCache implements ActionInputFileCache {
     String relPath = input.getExecPathString();
     return relPath.startsWith("/") ? relPath : new File(cwd, relPath).getPath();
   }
+
+  /** Container class for caching I/O around ActionInputs. */
+  private static class ActionInputMetadata {
+    private final ByteString digest;
+    private final long size;
+    private final IOException exceptionOnAccess;
+
+    /** Constructor for a successful lookup. */
+    ActionInputMetadata(ByteString digest, long size) {
+      this.digest = digest;
+      this.size = size;
+      this.exceptionOnAccess = null;
+    }
+
+    /** Constructor for a failed lookup, size will be 0. */
+    ActionInputMetadata(IOException exceptionOnAccess) {
+      this.exceptionOnAccess = exceptionOnAccess;
+      this.digest = null;
+      this.size = 0;
+    }
+
+    /** Returns digest or throws the exception encountered calculating it/ */
+    ByteString getDigest() throws IOException {
+      maybeRaiseException();
+      return digest;
+    }
+
+    /** Returns the size. */
+    long getSize() throws IOException {
+      maybeRaiseException();
+      return size;
+    }
+
+    private void maybeRaiseException() throws IOException {
+      if (exceptionOnAccess != null) {
+        throw exceptionOnAccess;
+      }
+    }
+  }
 }