diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index 26ccf19bc758f2..0f900b14b8bc72 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -273,7 +273,7 @@ private static FileArtifactValue create( return new DirectoryArtifactValue(path.getLastModifiedTime()); } if (digest == null) { - digest = DigestUtils.getDigestWithManualFallback(path, size, xattrProvider); + digest = DigestUtils.getDigestWithManualFallback(path, xattrProvider); } Preconditions.checkState(digest != null, path); return createForNormalFile(digest, proxy, size); diff --git a/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java b/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java index 1a7530ed99ab26..e92385a11679e4 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java +++ b/src/main/java/com/google/devtools/build/lib/exec/RunfilesTreeUpdater.java @@ -138,9 +138,8 @@ private void updateRunfilesTree( // an up-to-date check. if (!outputManifest.isSymbolicLink() && Arrays.equals( - DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(outputManifest, xattrProvider), - DigestUtils.getDigestWithManualFallbackWhenSizeUnknown( - inputManifest, xattrProvider))) { + DigestUtils.getDigestWithManualFallback(outputManifest, xattrProvider), + DigestUtils.getDigestWithManualFallback(inputManifest, xattrProvider))) { return; } } catch (IOException e) { diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java index 228107b4592877..50bc59082e0d43 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java @@ -176,8 +176,7 @@ static Digest computeDigest( // Try to obtain a digest from the filesystem. return builder .setHash( - HashCode.fromBytes( - DigestUtils.getDigestWithManualFallback(path, fileSize, xattrProvider)) + HashCode.fromBytes(DigestUtils.getDigestWithManualFallback(path, xattrProvider)) .toString()) .setSizeBytes(fileSize) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java b/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java index 271b3f534b782d..781adf4abb0a42 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java @@ -72,8 +72,7 @@ public Digest compute(Path file) throws IOException { } public Digest compute(Path file, long fileSize) throws IOException { - return buildDigest( - DigestUtils.getDigestWithManualFallback(file, fileSize, xattrProvider), fileSize); + return buildDigest(DigestUtils.getDigestWithManualFallback(file, xattrProvider), fileSize); } public Digest compute(VirtualActionInput input) throws IOException { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java index 1bc2f4c7c0ac6a..840068a35eb632 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java @@ -555,7 +555,7 @@ private FileArtifactValue constructFileArtifactValue( // possible to hit the digest cache - we probably already computed the digest for the // target during previous action execution. Path pathToDigest = isResolvedSymlink ? statAndValue.realPath() : statAndValue.pathNoFollow(); - actualDigest = DigestUtils.manuallyComputeDigest(pathToDigest, value.getSize()); + actualDigest = DigestUtils.manuallyComputeDigest(pathToDigest); } if (!isResolvedSymlink) { diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java index 77bd98d39ce3f5..ab801d680b56c1 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java @@ -150,40 +150,19 @@ public static CacheStats getCacheStats() { * #manuallyComputeDigest} to skip an additional attempt to obtain the fast digest. * * @param path Path of the file. - * @param fileSize Size of the file. Used to determine if digest calculation should be done - * serially or in parallel. Files larger than a certain threshold will be read serially, in - * order to avoid excessive disk seeks. */ - public static byte[] getDigestWithManualFallback( - Path path, long fileSize, XattrProvider xattrProvider) throws IOException { + public static byte[] getDigestWithManualFallback(Path path, XattrProvider xattrProvider) + throws IOException { byte[] digest = xattrProvider.getFastDigest(path); - return digest != null ? digest : manuallyComputeDigest(path, fileSize); - } - - /** - * Gets the digest of {@code path}, using a constant-time xattr call if the filesystem supports - * it, and calculating the digest manually otherwise. - * - *

Unlike {@link #getDigestWithManualFallback}, will not rate-limit manual digesting of files, - * so only use this method if the file size is truly unknown and you don't expect many concurrent - * manual digests of large files. - * - * @param path Path of the file. - */ - public static byte[] getDigestWithManualFallbackWhenSizeUnknown( - Path path, XattrProvider xattrProvider) throws IOException { - return getDigestWithManualFallback(path, -1, xattrProvider); + return digest != null ? digest : manuallyComputeDigest(path); } /** * Calculates the digest manually. * * @param path Path of the file. - * @param fileSize Size of the file. Used to determine if digest calculation should be done - * serially or in parallel. Files larger than a certain threshold will be read serially, in - * order to avoid excessive disk seeks. */ - public static byte[] manuallyComputeDigest(Path path, long fileSize) throws IOException { + public static byte[] manuallyComputeDigest(Path path) throws IOException { byte[] digest; // Attempt a cache lookup if the cache is enabled. @@ -199,7 +178,7 @@ public static byte[] manuallyComputeDigest(Path path, long fileSize) throws IOEx digest = path.getDigest(); - Preconditions.checkNotNull(digest, "Missing digest for %s (size %s)", path, fileSize); + Preconditions.checkNotNull(digest, "Missing digest for %s", path); if (cache != null) { cache.put(key, digest); } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java index d0fa82d2e59531..623a41566c7a39 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java @@ -717,8 +717,7 @@ public String getDirectoryDigest(XattrProvider xattrProvider) throws IOException } else { hasher.putChar('-'); } - hasher.putBytes( - DigestUtils.getDigestWithManualFallback(path, stat.getSize(), xattrProvider)); + hasher.putBytes(DigestUtils.getDigestWithManualFallback(path, xattrProvider)); } else if (stat.isDirectory()) { hasher.putChar('d').putUnencodedChars(path.getDirectoryDigest(xattrProvider)); } else if (stat.isSymbolicLink()) { @@ -732,8 +731,7 @@ public String getDirectoryDigest(XattrProvider xattrProvider) throws IOException } else { hasher.putChar('-'); } - hasher.putBytes( - DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(resolved, xattrProvider)); + hasher.putBytes(DigestUtils.getDigestWithManualFallback(resolved, xattrProvider)); } else { // link to a non-file: include the link itself in the hash hasher.putChar('l').putUnencodedChars(link.toString()); diff --git a/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java b/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java index 97001c1ed4bb36..357a16ae9a680b 100644 --- a/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java +++ b/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java @@ -42,9 +42,7 @@ public void testFoo() throws Exception { // Instead of actually trying to access this file, a call to getxattr() should be made. We // intercept this call and return a fake extended attribute value, thereby causing the checksum // computation to be skipped entirely. - assertThat( - DigestUtils.getDigestWithManualFallback( - absolutize("myfile"), 123, SyscallCache.NO_CACHE)) + assertThat(DigestUtils.getDigestWithManualFallback(absolutize("myfile"), SyscallCache.NO_CACHE)) .isEqualTo(FAKE_DIGEST); } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java index 3dfbf0eb968d22..05f36613ebdaaf 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/DigestUtilsTest.java @@ -75,12 +75,14 @@ protected byte[] getFastDigest(PathFragment path) throws IOException { TestThread thread1 = new TestThread( - () -> - DigestUtils.getDigestWithManualFallback(myFile1, fileSize1, SyscallCache.NO_CACHE)); + () -> { + var unused = DigestUtils.getDigestWithManualFallback(myFile1, SyscallCache.NO_CACHE); + }); TestThread thread2 = new TestThread( - () -> - DigestUtils.getDigestWithManualFallback(myFile2, fileSize2, SyscallCache.NO_CACHE)); + () -> { + var unused = DigestUtils.getDigestWithManualFallback(myFile2, SyscallCache.NO_CACHE); + }); thread1.start(); thread2.start(); if (!expectConcurrent) { // Synchronized case. @@ -120,14 +122,11 @@ protected byte[] getDigest(PathFragment path) throws IOException { Path file = tracingFileSystem.getPath("/file.txt"); FileSystemUtils.writeContentAsLatin1(file, "some contents"); - byte[] digest1 = - DigestUtils.getDigestWithManualFallback(file, file.getFileSize(), SyscallCache.NO_CACHE); + byte[] digest1 = DigestUtils.getDigestWithManualFallback(file, SyscallCache.NO_CACHE); assertThat(getFastDigestCounter.get()).isEqualTo(1); assertThat(getDigestCounter.get()).isEqualTo(1); - assertThat( - DigestUtils.getDigestWithManualFallback( - file, file.getFileSize(), SyscallCache.NO_CACHE)) + assertThat(DigestUtils.getDigestWithManualFallback(file, SyscallCache.NO_CACHE)) .isEqualTo(digest1); assertThat(getFastDigestCounter.get()).isEqualTo(2); assertThat(getDigestCounter.get()).isEqualTo(1); // Cached. @@ -151,6 +150,6 @@ protected byte[] getDigest(PathFragment path) { Path file = noDigestFileSystem.getPath("/f.txt"); FileSystemUtils.writeContentAsLatin1(file, "contents"); - assertThat(DigestUtils.manuallyComputeDigest(file, /*fileSize=*/ 8)).isEqualTo(digest); + assertThat(DigestUtils.manuallyComputeDigest(file)).isEqualTo(digest); } }