Skip to content

Commit

Permalink
[7.1.0] Add a profiler span for fetching repositories. (#20852)
Browse files Browse the repository at this point in the history 
This seems superfluous because we already have one for their Starlark
implementation function, but sometimes significant amount of time is
spent in RepositoryFunction just to determine that the repository is in
fact up-to-date.

In this case, it's useful to see in which repository the time is spent.

RELNOTES: None.
PiperOrigin-RevId: 597165396
Change-Id: I2c327450459a41dc7eec6ec2ac89c186cb43b34f
  • Loading branch information
@lberki
lberki authored Jan 11, 2024
1 parent 7fb091b commit 1b440b3
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 83 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public enum ProfilerTask {
PRESSURE_STALL_MEMORY("Memory pressure stall level"),
CONFLICT_CHECK("Conflict checking"),
DYNAMIC_LOCK("Acquiring dynamic execution output lock", Threshold.FIFTY_MILLIS),
REPOSITORY_FETCH("Fetching repository"),
UNKNOWN("Unknown event");

private static class Threshold {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/rules/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/repository:external_package_exception",
"//src/main/java/com/google/devtools/build/lib/repository:external_package_helper",
"//src/main/java/com/google/devtools/build/lib/repository:external_rule_not_found_exception",
Expand Down
183 changes: 100 additions & 83 deletions ...main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleFormatter;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.repository.ExternalPackageException;
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
import com.google.devtools.build.lib.repository.ExternalRuleNotFoundException;
Expand Down Expand Up @@ -115,10 +118,6 @@ public RepositoryDelegatorFunction(
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws InterruptedException, RepositoryFunctionException {
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
if (starlarkSemantics == null) {
return null;
}
RepositoryName repositoryName = (RepositoryName) skyKey.argument();
if (!repositoryName.isVisible()) {
return new NoRepositoryDirectoryValue(
Expand All @@ -127,105 +126,123 @@ public SkyValue compute(SkyKey skyKey, Environment env)
repositoryName.getName(), repositoryName.getOwnerRepoDisplayString()));
}

Path repoRoot =
RepositoryFunction.getExternalRepositoryDirectory(directories)
.getRelative(repositoryName.getName());
Map<RepositoryName, PathFragment> overrides = REPOSITORY_OVERRIDES.get(env);
if (Preconditions.checkNotNull(overrides).containsKey(repositoryName)) {
DigestWriter.clearMarkerFile(directories, repositoryName);
return setupOverride(overrides.get(repositoryName), env, repoRoot, repositoryName.getName());
}
try (SilentCloseable c =
Profiler.instance().profile(ProfilerTask.REPOSITORY_FETCH, repositoryName.toString())) {
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
if (starlarkSemantics == null) {
return null;
}
Path repoRoot =
RepositoryFunction.getExternalRepositoryDirectory(directories)
.getRelative(repositoryName.getName());
Map<RepositoryName, PathFragment> overrides = REPOSITORY_OVERRIDES.get(env);
if (Preconditions.checkNotNull(overrides).containsKey(repositoryName)) {
DigestWriter.clearMarkerFile(directories, repositoryName);
return setupOverride(
overrides.get(repositoryName), env, repoRoot, repositoryName.getName());
}

Rule rule = null;
if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
// Tries to get a repository rule instance from Bzlmod generated repos.
SkyKey key = BzlmodRepoRuleValue.key(repositoryName);
BzlmodRepoRuleValue value = (BzlmodRepoRuleValue) env.getValue(key);
Rule rule = getRepositoryRule(env, repositoryName, starlarkSemantics);
if (env.valuesMissing()) {
return null;
}
if (value != BzlmodRepoRuleValue.REPO_RULE_NOT_FOUND_VALUE) {
rule = value.getRule();

if (rule == null) {
return new NoRepositoryDirectoryValue(
String.format("Repository '%s' is not defined", repositoryName.getCanonicalForm()));
}
}

if (rule == null) {
// fallback to look up the repository in the WORKSPACE file.
try {
rule = getRepoRuleFromWorkspace(repositoryName, env);
RepositoryFunction handler = getHandler(rule);
if (handler == null) {
// If we refer to a non repository rule then the repository does not exist.
return new NoRepositoryDirectoryValue(
String.format("'%s' is not a repository rule", repositoryName.getCanonicalForm()));
}

DigestWriter digestWriter = new DigestWriter(directories, repositoryName, rule);
if (shouldUseCachedRepos(env, handler, repoRoot, rule)) {
// Make sure marker file is up-to-date; correctly describes the current repository state
byte[] markerHash = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env);
if (env.valuesMissing()) {
return null;
}
} catch (NoSuchRepositoryException e) {
return new NoRepositoryDirectoryValue(
String.format("Repository '%s' is not defined", repositoryName.getCanonicalForm()));
if (markerHash != null) {
return RepositoryDirectoryValue.builder().setPath(repoRoot).setDigest(markerHash).build();
}
}
}

RepositoryFunction handler = getHandler(rule);
if (handler == null) {
// If we refer to a non repository rule then the repository does not exist.
return new NoRepositoryDirectoryValue(
String.format("'%s' is not a repository rule", repositoryName.getCanonicalForm()));
}

DigestWriter digestWriter = new DigestWriter(directories, repositoryName, rule);
if (shouldUseCachedRepos(env, handler, repoRoot, rule)) {
// Make sure marker file is up-to-date; correctly describes the current repository state
byte[] markerHash = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env);
if (env.valuesMissing()) {
return null;
/* At this point: This is a force fetch, a local repository, OR The repository cache is old or
didn't exist. In any of those cases, we initiate the fetching process UNLESS this is offline
mode (fetching is disabled) */
if (isFetch.get()) {
// Fetching a repository is a long-running operation that can easily be interrupted. If it
// is and the marker file exists on disk, a new call of this method may treat this
// repository as valid even though it is in an inconsistent state. Clear the marker file and
// only recreate it after fetching is done to prevent this scenario.
DigestWriter.clearMarkerFile(directories, repositoryName);
RepositoryDirectoryValue.Builder builder =
fetchRepository(skyKey, repoRoot, env, digestWriter.getMarkerData(), handler, rule);
if (builder == null) {
return null;
}
// No new Skyframe dependencies must be added between calling the repository implementation
// and writing the marker file because if they aren't computed, it would cause a Skyframe
// restart thus calling the possibly very slow (networking, decompression...) fetch()
// operation again. So we write the marker file here immediately.
byte[] digest = digestWriter.writeMarkerFile();
return builder.setDigest(digest).build();
}
if (markerHash != null) {
return RepositoryDirectoryValue.builder().setPath(repoRoot).setDigest(markerHash).build();

if (!repoRoot.exists()) {
// The repository isn't on the file system, there is nothing we can do.
throw new RepositoryFunctionException(
new IOException(
"to fix, run\n\tbazel fetch //...\nExternal repository "
+ repositoryName
+ " not found and fetching repositories is disabled."),
Transience.TRANSIENT);
}

// Try to build with whatever is on the file system and emit a warning.
env.getListener()
.handle(
Event.warn(
rule.getLocation(),
String.format(
"External repository '%s' is not up-to-date and fetching is disabled. To"
+ " update, run the build without the '--nofetch' command line option.",
rule.getName())));

return RepositoryDirectoryValue.builder()
.setPath(repoRoot)
.setFetchingDelayed()
.setDigest(new Fingerprint().digestAndReset())
.build();
}
}

/* At this point: This is a force fetch, a local repository, OR The repository cache is old or
didn't exist. In any of those cases, we initiate the fetching process UNLESS this is offline
mode (fetching is disabled) */
if (isFetch.get()) {
// Fetching a repository is a long-running operation that can easily be interrupted. If it is
// and the marker file exists on disk, a new call of this method may treat this repository as
// valid even though it is in an inconsistent state. Clear the marker file and only recreate
// it after fetching is done to prevent this scenario.
DigestWriter.clearMarkerFile(directories, repositoryName);
RepositoryDirectoryValue.Builder builder =
fetchRepository(skyKey, repoRoot, env, digestWriter.getMarkerData(), handler, rule);
if (builder == null) {
@Nullable
private Rule getRepositoryRule(
Environment env, RepositoryName repositoryName, StarlarkSemantics starlarkSemantics)
throws InterruptedException, RepositoryFunctionException {
if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
// Tries to get a repository rule instance from Bzlmod generated repos.
SkyKey key = BzlmodRepoRuleValue.key(repositoryName);
BzlmodRepoRuleValue value = (BzlmodRepoRuleValue) env.getValue(key);
if (env.valuesMissing()) {
return null;
}
// No new Skyframe dependencies must be added between calling the repository implementation
// and writing the marker file because if they aren't computed, it would cause a Skyframe
// restart thus calling the possibly very slow (networking, decompression...) fetch()
// operation again. So we write the marker file here immediately.
byte[] digest = digestWriter.writeMarkerFile();
return builder.setDigest(digest).build();
if (value != BzlmodRepoRuleValue.REPO_RULE_NOT_FOUND_VALUE) {
return value.getRule();
}
}

if (!repoRoot.exists()) {
// The repository isn't on the file system, there is nothing we can do.
throw new RepositoryFunctionException(
new IOException(
"to fix, run\n\tbazel fetch //...\nExternal repository "
+ repositoryName
+ " not found and fetching repositories is disabled."),
Transience.TRANSIENT);
// fallback to look up the repository in the WORKSPACE file.
try {
return getRepoRuleFromWorkspace(repositoryName, env);
} catch (NoSuchRepositoryException e) {
return null;
}

// Try to build with whatever is on the file system and emit a warning.
env.getListener()
.handle(Event.warn(rule.getLocation(),
String.format(
"External repository '%s' is not up-to-date and fetching is disabled. To update, "
+ "run the build without the '--nofetch' command line option.",
rule.getName())));

return RepositoryDirectoryValue.builder()
.setPath(repoRoot)
.setFetchingDelayed()
.setDigest(new Fingerprint().digestAndReset())
.build();
}

@Nullable
Expand Down

0 comments on commit 1b440b3

Please sign in to comment.