Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.1.0] Add a profiler span for fetching repositories. #20852

Merged
merged 1 commit into from
Jan 11, 2024
Merged

[7.1.0] Add a profiler span for fetching repositories. #20852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading