Skip to content

Commit

Permalink
Add #getFastDigest method to SyscallCache and use it in DigestUtils. …
Browse files Browse the repository at this point in the history
…Thread the real per-build SyscallCache through to everywhere that calls into DigestUtils.

This leaves FileStateValue's call of #getFastDigest as the main undelegated one. A follow-up change will get rid of that usage too.

There should be no observable difference from this change: the only new actual usage of SyscallCache is in DigestUtils, which calls getFastDigest, which just delegates back to the Path for now.

PiperOrigin-RevId: 424972494
  • Loading branch information
janakdr authored and copybara-github committed Jan 28, 2022
1 parent 8174262 commit 491e441
Show file tree
Hide file tree
Showing 66 changed files with 323 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,7 @@ public DiscoveredModulesPruner getDiscoveredModulesPruner() {
return discoveredModulesPruner;
}

/**
* This only exists for loose header checking (and shouldn't exist at all).
*
* <p>Do NOT use from any other place.
*/
/** This only exists for loose header checking and as a helper for digest computations. */
public SyscallCache getSyscallCache() {
return syscallCache;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.skyframe.SkyValue;
import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -189,8 +190,8 @@ interface Singleton {}
@SerializationConstant
public static final FileArtifactValue OMITTED_FILE_MARKER = new OmittedFileValue();

public static FileArtifactValue createForSourceArtifact(Artifact artifact, FileValue fileValue)
throws IOException {
public static FileArtifactValue createForSourceArtifact(
Artifact artifact, FileValue fileValue, SyscallCache syscallCache) throws IOException {
// Artifacts with known generating actions should obtain the derived artifact's SkyValue
// from the generating action, instead.
Preconditions.checkState(!artifact.hasKnownGeneratingAction());
Expand All @@ -201,7 +202,8 @@ public static FileArtifactValue createForSourceArtifact(Artifact artifact, FileV
isFile,
isFile ? fileValue.getSize() : 0,
isFile ? fileValue.realFileStateValue().getContentsProxy() : null,
isFile ? fileValue.getDigest() : null);
isFile ? fileValue.getDigest() : null,
syscallCache);
}

public static FileArtifactValue createFromInjectedDigest(
Expand All @@ -219,16 +221,27 @@ public static FileArtifactValue createForTesting(Path path) throws IOException {
// Caution: there's a race condition between stating the file and computing the digest. We need
// to stat first, since we're using the stat to detect changes. We follow symlinks here to be
// consistent with getDigest.
return createFromStat(path, path.stat(Symlinks.FOLLOW));
return createFromStat(path, path.stat(Symlinks.FOLLOW), SyscallCache.NO_CACHE);
}

public static FileArtifactValue createFromStat(Path path, FileStatus stat) throws IOException {
public static FileArtifactValue createFromStat(
Path path, FileStatus stat, SyscallCache syscallCache) throws IOException {
return create(
path, stat.isFile(), stat.getSize(), FileContentsProxy.create(stat), /*digest=*/ null);
path,
stat.isFile(),
stat.getSize(),
FileContentsProxy.create(stat),
/*digest=*/ null,
syscallCache);
}

private static FileArtifactValue create(
Path path, boolean isFile, long size, FileContentsProxy proxy, @Nullable byte[] digest)
Path path,
boolean isFile,
long size,
FileContentsProxy proxy,
@Nullable byte[] digest,
SyscallCache syscallCache)
throws IOException {
if (!isFile) {
// In this case, we need to store the mtime because the action cache uses mtime for
Expand All @@ -237,7 +250,7 @@ private static FileArtifactValue create(
return new DirectoryArtifactValue(path.getLastModifiedTime());
}
if (digest == null) {
digest = DigestUtils.getDigestWithManualFallback(path, size);
digest = DigestUtils.getDigestWithManualFallback(path, size, syscallCache);
}
Preconditions.checkState(digest != null, path);
return createForNormalFile(digest, proxy, size);
Expand Down Expand Up @@ -275,9 +288,9 @@ public static FileArtifactValue createForNormalFile(
* Create a FileArtifactValue using the {@link Path} and size. FileArtifactValue#create will
* handle getting the digest using the Path and size values.
*/
public static FileArtifactValue createForNormalFileUsingPath(Path path, long size)
throws IOException {
return create(path, /*isFile=*/ true, size, /*proxy=*/ null, /*digest=*/ null);
public static FileArtifactValue createForNormalFileUsingPath(
Path path, long size, SyscallCache syscallCache) throws IOException {
return create(path, /*isFile=*/ true, size, /*proxy=*/ null, /*digest=*/ null, syscallCache);
}

public static FileArtifactValue createForDirectoryWithHash(byte[] digest) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ java_library(
srcs = ["ResolvedEvent.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/vfs",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {

ProcessWrapper processWrapper = ProcessWrapper.fromCommandEnvironment(env);
starlarkRepositoryFunction.setProcessWrapper(processWrapper);
starlarkRepositoryFunction.setSyscallCache(env.getSyscallCache());
singleExtensionEvalFunction.setProcessWrapper(processWrapper);

RepositoryOptions repoOptions = env.getOptions().getOptions(RepositoryOptions.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
package com.google.devtools.build.lib.bazel;

import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.vfs.SyscallCache;

/** Interface for events reporting information to be added to a resolved file. */
public interface ResolvedEvent extends ExtendedEventHandler.ProgressLike {
/** The name of the resolved entity, e.g., the name of an external repository. */
String getName();

/** The entry for the list of resolved Information. */
Object getResolvedInformation();
Object getResolvedInformation(SyscallCache syscallCache);
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ private void initOutputs(CommandEnvironment env) throws IOException {

spawnLogContext =
new SpawnLogContext(
env.getExecRoot(), outStream, env.getOptions().getOptions(RemoteOptions.class));
env.getExecRoot(),
outStream,
env.getOptions().getOptions(RemoteOptions.class),
env.getSyscallCache());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.InterruptedFailureDetails;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyKey;
Expand Down Expand Up @@ -263,7 +264,7 @@ public String getName() {
}

@Override
public Object getResolvedInformation() {
public Object getResolvedInformation(SyscallCache syscallCache) {
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, "bind")
.put(
Expand Down Expand Up @@ -295,7 +296,7 @@ public String getName() {
}

@Override
public Object getResolvedInformation() {
public Object getResolvedInformation(SyscallCache syscallCache) {
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, ruleName)
.put(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.util.CPU;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
Expand Down Expand Up @@ -85,7 +86,7 @@ public String getName() {
}

@Override
public Object getResolvedInformation() {
public Object getResolvedInformation(SyscallCache syscallCache) {
String repr = String.format("local_config_platform(name = '%s')", name);
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, LocalConfigPlatformRule.NAME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.SyscallCache;
import java.io.IOException;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -164,13 +165,13 @@ public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory
* Ensure that the {@code resolvedInformation} and the {@code directoryDigest} fields are
* initialized properly. Does nothing, if the values are computed already.
*/
private synchronized void finalizeResolvedInformation() {
private synchronized void finalizeResolvedInformation(SyscallCache syscallCache) {
if (resolvedInformation != null) {
return;
}
String digest = "[unavailable]";
try {
digest = outputDirectory.getDirectoryDigest();
digest = outputDirectory.getDirectoryDigest(syscallCache);
repositoryBuilder.put(OUTPUT_TREE_HASH, digest);
} catch (IOException e) {
// Digest not available, but we still have to report that a repository rule
Expand All @@ -190,8 +191,8 @@ private synchronized void finalizeResolvedInformation() {
* Returns the entry for the given rule invocation in a format suitable for WORKSPACE.resolved.
*/
@Override
public Object getResolvedInformation() {
finalizeResolvedInformation();
public Object getResolvedInformation(SyscallCache syscallCache) {
finalizeResolvedInformation(syscallCache);
return resolvedInformation;
}

Expand All @@ -201,8 +202,8 @@ public String getName() {
return name;
}

public String getDirectoryDigest() {
finalizeResolvedInformation();
public String getDirectoryDigest(SyscallCache syscallCache) {
finalizeResolvedInformation(syscallCache);
return directoryDigest;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.common.options.OptionsBase;
import java.io.File;
import java.io.IOException;
Expand All @@ -41,6 +42,7 @@ public final class RepositoryResolvedModule extends BlazeModule {
private Map<String, Object> resolvedValues;
private String resolvedFile;
private ImmutableList<String> orderedNames;
private SyscallCache syscallCache;

@Override
public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
Expand All @@ -61,6 +63,7 @@ public void beforeCommand(CommandEnvironment env) {
} else {
this.resolvedFile = null;
}
this.syscallCache = env.getSyscallCache();
}

@Override
Expand Down Expand Up @@ -97,7 +100,7 @@ public void repositoryOrderEvent(RepositoryOrderEvent event) {
@Subscribe
public void resolved(ResolvedEvent event) {
if (resolvedValues != null) {
resolvedValues.put(event.getName(), event.getResolvedInformation());
resolvedValues.put(event.getName(), event.getResolvedInformation(syscallCache));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
Expand All @@ -66,6 +67,7 @@ public class StarlarkRepositoryFunction extends RepositoryFunction {
private double timeoutScaling = 1.0;
@Nullable private ProcessWrapper processWrapper = null;
@Nullable private RepositoryRemoteExecutor repositoryRemoteExecutor;
@Nullable private SyscallCache syscallCache;

public StarlarkRepositoryFunction(DownloadManager downloadManager) {
this.downloadManager = downloadManager;
Expand All @@ -79,6 +81,10 @@ public void setProcessWrapper(@Nullable ProcessWrapper processWrapper) {
this.processWrapper = processWrapper;
}

public void setSyscallCache(SyscallCache syscallCache) {
this.syscallCache = syscallCache;
}

static String describeSemantics(StarlarkSemantics semantics) {
// Here we use the hash code provided by AutoValue. This is unique, as long
// as the number of bits in the StarlarkSemantics is small enough. We will have to
Expand Down Expand Up @@ -236,7 +242,7 @@ public RepositoryDirectoryValue.Builder fetch(
if (verificationRules.contains(ruleClass)) {
String expectedHash = resolvedHashes.get(rule.getName());
if (expectedHash != null) {
String actualHash = resolved.getDirectoryDigest();
String actualHash = resolved.getDirectoryDigest(syscallCache);
if (!expectedHash.equals(actualHash)) {
throw new RepositoryFunctionException(
new IOException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.vfs.DigestUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.SyscallCache;
import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -66,7 +67,8 @@ private static void updateRunfilesTree(
BinTools binTools,
ImmutableMap<String, String> env,
OutErr outErr,
boolean enableRunfiles)
boolean enableRunfiles,
SyscallCache syscallCache)
throws IOException, ExecException, InterruptedException {
Path runfilesDirPath = execRoot.getRelative(runfilesDir);
Path inputManifest = RunfilesSupport.inputManifestPath(runfilesDirPath);
Expand All @@ -82,8 +84,9 @@ private static void updateRunfilesTree(
// an up-to-date check.
if (!outputManifest.isSymbolicLink()
&& Arrays.equals(
DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(outputManifest),
DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(inputManifest))) {
DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(outputManifest, syscallCache),
DigestUtils.getDigestWithManualFallbackWhenSizeUnknown(
inputManifest, syscallCache))) {
return;
}
} catch (IOException e) {
Expand Down Expand Up @@ -131,7 +134,8 @@ public void updateRunfilesDirectory(
RunfilesSupplier runfilesSupplier,
BinTools binTools,
ImmutableMap<String, String> env,
OutErr outErr)
OutErr outErr,
SyscallCache syscallCache)
throws ExecException, IOException, InterruptedException {
for (Map.Entry<PathFragment, Map<PathFragment, Artifact>> runfiles :
runfilesSupplier.getMappings().entrySet()) {
Expand All @@ -154,7 +158,8 @@ public void updateRunfilesDirectory(
binTools,
env,
outErr,
runfilesSupplier.isRunfileLinksEnabled(runfilesDir));
runfilesSupplier.isRunfileLinksEnabled(runfilesDir),
syscallCache);
}
} finally {
decrementRefcnt(runfilesDir);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.SyscallCache;
import java.io.IOException;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
Expand All @@ -46,8 +47,10 @@ public class SingleBuildFileCache implements MetadataProvider {
// unlikely that this default will adversely affect memory in most cases.
.initialCapacity(10000)
.build();
private final SyscallCache syscallCache;

public SingleBuildFileCache(String cwd, FileSystem fs) {
public SingleBuildFileCache(String cwd, FileSystem fs, SyscallCache syscallCache) {
this.syscallCache = syscallCache;
this.execRoot = fs.getPath(cwd);
}

Expand All @@ -60,7 +63,12 @@ public FileArtifactValue getMetadata(ActionInput input) throws IOException {
Path path = ActionInputHelper.toInputPath(input, execRoot);
FileArtifactValue metadata;
try {
metadata = FileArtifactValue.createFromStat(path, path.stat(Symlinks.FOLLOW));
metadata =
FileArtifactValue.createFromStat(
path,
// TODO(b/199940216): should we use syscallCache here since caching anyway?
path.stat(Symlinks.FOLLOW),
syscallCache);
} catch (IOException e) {
return new ActionInputMetadata(input, e);
}
Expand Down
Loading

0 comments on commit 491e441

Please sign in to comment.