Skip to content

Commit

Permalink
Replace more usages of DigestHashFunction#getDefaultUnchecked() by ex…
Browse files Browse the repository at this point in the history
…plicit statements.

PiperOrigin-RevId: 330583533
  • Loading branch information
janakdr authored and copybara-github committed Sep 8, 2020
1 parent 8faeb9e commit 3af783a
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -50,20 +50,19 @@
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
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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -116,7 +116,7 @@ private BzlLoadFunction(

public static BzlLoadFunction create(
PackageFactory packageFactory,
DigestHashFunction digestHashFunction,
HashFunction hashFunction,
Cache<ASTFileLookupValue.Key, ASTFileLookupValue> astFileLookupValueCache) {
return new BzlLoadFunction(
packageFactory,
Expand All @@ -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);
}

Expand Down Expand Up @@ -986,18 +985,18 @@ 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.)
private final Cache<ASTFileLookupValue.Key, ASTFileLookupValue> astFileLookupValueCache;

private InliningAndCachingASTManager(
PackageFactory packageFactory,
DigestHashFunction digestHashFunction,
HashFunction hashFunction,
Cache<ASTFileLookupValue.Key, ASTFileLookupValue> astFileLookupValueCache) {
this.packageFactory = packageFactory;
this.digestHashFunction = digestHashFunction;
this.hashFunction = hashFunction;
this.astFileLookupValueCache = astFileLookupValueCache;
}

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -488,9 +487,7 @@ private ImmutableMap<SkyFunctionName, SkyFunction> 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());
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -142,7 +142,7 @@ public Diff getDiff(WalkableGraph fromGraph, Version fromVersion, Version toVers
private final AtomicReference<PathPackageLocator> 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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -155,6 +156,7 @@ public void setupDelegator() throws Exception {
.getPackageFactoryBuilderForTesting(directories)
.build(ruleClassProvider, fileSystem);

HashFunction hashFunction = fileSystem.getDigestFunction().getHashFunction();
MemoizingEvaluator evaluator =
new InMemoryMemoizingEvaluator(
ImmutableMap.<SkyFunctionName, SkyFunction>builder()
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3af783a

Please sign in to comment.