From 3af783a557cf7c1b5e1ba37bdd33941b70ffadd2 Mon Sep 17 00:00:00 2001 From: janakr Date: Tue, 8 Sep 2020 14:12:14 -0700 Subject: [PATCH] Replace more usages of DigestHashFunction#getDefaultUnchecked() by explicit statements. PiperOrigin-RevId: 330583533 --- .../build/lib/skyframe/ASTFileLookupFunction.java | 15 +++++++-------- .../build/lib/skyframe/BzlLoadFunction.java | 15 +++++++-------- .../build/lib/skyframe/SkyframeExecutor.java | 8 ++------ .../skyframe/packages/AbstractPackageLoader.java | 12 +++++------- .../rules/repository/RepositoryDelegatorTest.java | 8 ++++---- 5 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java index 79ce8a03ed1f40..b378532d248d24 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.hash.HashFunction; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; @@ -25,7 +26,6 @@ import com.google.devtools.build.lib.syntax.Resolver; import com.google.devtools.build.lib.syntax.StarlarkFile; import com.google.devtools.build.lib.syntax.StarlarkSemantics; -import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.RootedPath; @@ -50,12 +50,11 @@ public class ASTFileLookupFunction implements SkyFunction { private final PackageFactory packageFactory; - private final DigestHashFunction digestHashFunction; + private final HashFunction hashFunction; - public ASTFileLookupFunction( - PackageFactory packageFactory, DigestHashFunction digestHashFunction) { + public ASTFileLookupFunction(PackageFactory packageFactory, HashFunction hashFunction) { this.packageFactory = packageFactory; - this.digestHashFunction = digestHashFunction; + this.hashFunction = hashFunction; } @Override @@ -63,7 +62,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException, InterruptedException { try { return computeInline( - (ASTFileLookupValue.Key) skyKey.argument(), env, packageFactory, digestHashFunction); + (ASTFileLookupValue.Key) skyKey.argument(), env, packageFactory, hashFunction); } catch (ASTLookupFailedException e) { throw new ASTLookupFunctionException(e); } @@ -73,7 +72,7 @@ static ASTFileLookupValue computeInline( ASTFileLookupValue.Key key, Environment env, PackageFactory packageFactory, - DigestHashFunction digestHashFunction) + HashFunction digestHashFunction) throws ASTLookupFailedException, InterruptedException { byte[] bytes; byte[] digest; @@ -133,7 +132,7 @@ static ASTFileLookupValue computeInline( // Compute digest if we didn't already get it from a fileValue. if (digest == null) { - digest = digestHashFunction.getHashFunction().hashBytes(bytes).asBytes(); + digest = digestHashFunction.hashBytes(bytes).asBytes(); } StarlarkSemantics semantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java index e28329197fe808..abe9f8aefcad67 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java @@ -22,6 +22,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.flogger.GoogleLogger; +import com.google.common.hash.HashFunction; import com.google.devtools.build.lib.actions.InconsistentFilesystemException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; @@ -54,7 +55,6 @@ import com.google.devtools.build.lib.syntax.Statement; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Pair; -import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.RecordingSkyFunctionEnvironment; import com.google.devtools.build.skyframe.SkyFunction; @@ -116,7 +116,7 @@ private BzlLoadFunction( public static BzlLoadFunction create( PackageFactory packageFactory, - DigestHashFunction digestHashFunction, + HashFunction hashFunction, Cache astFileLookupValueCache) { return new BzlLoadFunction( packageFactory, @@ -143,8 +143,7 @@ public static BzlLoadFunction create( // just a temporary thing for bzl execution. Retaining it forever is pure waste. // (b) The memory overhead of the extra Skyframe node and edge per bzl file is pure // waste. - new InliningAndCachingASTManager( - packageFactory, digestHashFunction, astFileLookupValueCache), + new InliningAndCachingASTManager(packageFactory, hashFunction, astFileLookupValueCache), /*cachedBzlLoadDataManager=*/ null); } @@ -986,7 +985,7 @@ public void doneWithASTFileLookupValue(ASTFileLookupValue.Key key) {} */ private static class InliningAndCachingASTManager implements ASTManager { private final PackageFactory packageFactory; - private final DigestHashFunction digestHashFunction; + private final HashFunction hashFunction; // We keep a cache of ASTFileLookupValues that have been computed but whose corresponding // BzlLoadValue has not yet completed. This avoids repeating the ASTFileLookupValue work in case // of Skyframe restarts. (If we weren't inlining, Skyframe would cache this for us.) @@ -994,10 +993,10 @@ private static class InliningAndCachingASTManager implements ASTManager { private InliningAndCachingASTManager( PackageFactory packageFactory, - DigestHashFunction digestHashFunction, + HashFunction hashFunction, Cache astFileLookupValueCache) { this.packageFactory = packageFactory; - this.digestHashFunction = digestHashFunction; + this.hashFunction = hashFunction; this.astFileLookupValueCache = astFileLookupValueCache; } @@ -1007,7 +1006,7 @@ public ASTFileLookupValue getASTFileLookupValue(ASTFileLookupValue.Key key, Envi throws ASTLookupFailedException, InterruptedException { ASTFileLookupValue value = astFileLookupValueCache.getIfPresent(key); if (value == null) { - value = ASTFileLookupFunction.computeInline(key, env, packageFactory, digestHashFunction); + value = ASTFileLookupFunction.computeInline(key, env, packageFactory, hashFunction); if (value != null) { astFileLookupValueCache.put(key, value); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index a2b3c0500f6538..982d9abd22d39c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -165,7 +165,6 @@ import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.ResourceUsage; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; -import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.ModifiedFileSet; @@ -488,9 +487,7 @@ private ImmutableMap skyFunctions(PackageFactory p buildFilesByPriority, externalPackageHelper)); map.put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()); - map.put( - SkyFunctions.AST_FILE_LOOKUP, - new ASTFileLookupFunction(pkgFactory, DigestHashFunction.getDefaultUnchecked())); + map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(pkgFactory, getHashFunction())); map.put(SkyFunctions.STARLARK_BUILTINS, new StarlarkBuiltinsFunction(pkgFactory)); map.put(SkyFunctions.BZL_LOAD, newBzlLoadFunction(ruleClassProvider, pkgFactory)); map.put(SkyFunctions.GLOB, newGlobFunction()); @@ -650,8 +647,7 @@ protected BzlLoadFunction getBzlLoadFunctionForInliningPackageAndWorkspaceNodes( protected SkyFunction newBzlLoadFunction( RuleClassProvider ruleClassProvider, PackageFactory pkgFactory) { - return BzlLoadFunction.create( - this.pkgFactory, DigestHashFunction.getDefaultUnchecked(), astFileLookupValueCache); + return BzlLoadFunction.create(this.pkgFactory, getHashFunction(), astFileLookupValueCache); } protected PerBuildSyscallCache newPerBuildSyscallCache(int concurrencyLevel) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 07940d16d6489c..ac00dc7d67ab78 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.EventBus; +import com.google.common.hash.HashFunction; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -81,7 +82,6 @@ import com.google.devtools.build.lib.syntax.StarlarkFile; import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; -import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; @@ -142,7 +142,7 @@ public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVers private final AtomicReference pkgLocatorRef; protected final ExternalFilesHelper externalFilesHelper; protected final BlazeDirectories directories; - private final DigestHashFunction digestHashFunction; + private final HashFunction hashFunction; private final int legacyGlobbingThreads; @VisibleForTesting final ForkJoinPool forkJoinPoolForLegacyGlobbing; private final int skyframeThreads; @@ -277,7 +277,7 @@ public final PackageLoader build() { "package-loader-globbing-pool", builder.legacyGlobbingThreads); this.skyframeThreads = builder.skyframeThreads; this.directories = builder.directories; - this.digestHashFunction = builder.workspaceDir.getFileSystem().getDigestFunction(); + this.hashFunction = builder.workspaceDir.getFileSystem().getDigestFunction().getHashFunction(); this.externalFilesHelper = builder.externalFilesHelper; @@ -485,13 +485,11 @@ public Path getBuildFileForPackage(PackageIdentifier packageName) { new IgnoredPackagePrefixesFunction( /*ignoredPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT)) .put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()) - .put( - SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(pkgFactory, digestHashFunction)) + .put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(pkgFactory, hashFunction)) .put(SkyFunctions.STARLARK_BUILTINS, new StarlarkBuiltinsFunction(pkgFactory)) .put( SkyFunctions.BZL_LOAD, - BzlLoadFunction.create( - pkgFactory, digestHashFunction, CacheBuilder.newBuilder().build())) + BzlLoadFunction.create(pkgFactory, hashFunction, CacheBuilder.newBuilder().build())) .put(SkyFunctions.WORKSPACE_NAME, new WorkspaceNameFunction()) .put(SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(ruleClassProvider)) .put( diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index 566db7d5ed4bfb..aadc37d43ecefc 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.hash.HashFunction; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -155,6 +156,7 @@ public void setupDelegator() throws Exception { .getPackageFactoryBuilderForTesting(directories) .build(ruleClassProvider, fileSystem); + HashFunction hashFunction = fileSystem.getDigestFunction().getHashFunction(); MemoizingEvaluator evaluator = new InMemoryMemoizingEvaluator( ImmutableMap.builder() @@ -196,13 +198,11 @@ public void setupDelegator() throws Exception { .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) .put( SkyFunctions.AST_FILE_LOOKUP, - new ASTFileLookupFunction(pkgFactory, fileSystem.getDigestFunction())) + new ASTFileLookupFunction(pkgFactory, hashFunction)) .put( SkyFunctions.BZL_LOAD, BzlLoadFunction.create( - pkgFactory, - fileSystem.getDigestFunction(), - CacheBuilder.newBuilder().build())) + pkgFactory, hashFunction, CacheBuilder.newBuilder().build())) .put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()) .put( SkyFunctions.IGNORED_PACKAGE_PREFIXES,