From cfbf2765dd22a1e4464081c81fa94a3aed1b869c Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 10 Jan 2025 06:24:01 -0800 Subject: [PATCH] Add more Bzlmod-related profiler spans This fills in a few gaps in the timing profile. Closes #24762. PiperOrigin-RevId: 714028550 Change-Id: I5772b59c5d3b6d73306ea5d8380169465f348f27 --- .../bazel/bzlmod/BazelDepGraphFunction.java | 50 +++++++++++-------- .../bzlmod/SingleExtensionEvalFunction.java | 7 ++- .../devtools/build/lib/bazel/commands/BUILD | 1 + .../build/lib/bazel/commands/ModCommand.java | 14 ++++-- 4 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java index 009b0cdc9f85ec..d699751bb0bf9e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java @@ -33,6 +33,9 @@ import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.packages.LabelConverter; +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.server.FailureDetails.ExternalDeps.Code; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; @@ -63,29 +66,32 @@ public SkyValue compute(SkyKey skyKey, Environment env) } var depGraph = selectionResult.getResolvedDepGraph(); - ImmutableBiMap canonicalRepoNameLookup = - computeCanonicalRepoNameLookup(depGraph); - ImmutableTable extensionUsagesById; - try { - extensionUsagesById = getExtensionUsagesById(depGraph, canonicalRepoNameLookup.inverse()); - } catch (ExternalDepsException e) { - throw new BazelDepGraphFunctionException(e, Transience.PERSISTENT); - } - - ImmutableBiMap extensionUniqueNames = - calculateUniqueNameForUsedExtensionId(extensionUsagesById); + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.BZLMOD, "finalize dep graph")) { + ImmutableBiMap canonicalRepoNameLookup = + computeCanonicalRepoNameLookup(depGraph); + ImmutableTable extensionUsagesById; + try { + extensionUsagesById = getExtensionUsagesById(depGraph, canonicalRepoNameLookup.inverse()); + } catch (ExternalDepsException e) { + throw new BazelDepGraphFunctionException(e, Transience.PERSISTENT); + } - return BazelDepGraphValue.create( - depGraph, - canonicalRepoNameLookup, - depGraph.values().stream().map(AbridgedModule::from).collect(toImmutableList()), - extensionUsagesById, - extensionUniqueNames.inverse(), - resolveRepoOverrides( - depGraph, - extensionUsagesById, - extensionUniqueNames.inverse(), - canonicalRepoNameLookup)); + ImmutableBiMap extensionUniqueNames = + calculateUniqueNameForUsedExtensionId(extensionUsagesById); + + return BazelDepGraphValue.create( + depGraph, + canonicalRepoNameLookup, + depGraph.values().stream().map(AbridgedModule::from).collect(toImmutableList()), + extensionUsagesById, + extensionUniqueNames.inverse(), + resolveRepoOverrides( + depGraph, + extensionUsagesById, + extensionUniqueNames.inverse(), + canonicalRepoNameLookup)); + } } private static ImmutableTable diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 68c45e3dd773f3..3194f3f8e2f968 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -31,6 +31,9 @@ import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; +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.rules.repository.NeedsSkyframeRestartException; import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; import com.google.devtools.build.lib.runtime.ProcessWrapper; @@ -148,7 +151,9 @@ public SkyValue compute(SkyKey skyKey, Environment env) lockedExtension = lockedExtensionMap == null ? null : lockedExtensionMap.get(extension.getEvalFactors()); if (lockedExtension != null) { - try { + try (SilentCloseable c = + Profiler.instance() + .profile(ProfilerTask.BZLMOD, () -> "check lockfile for " + extensionId)) { SingleExtensionValue singleExtensionValue = tryGettingValueFromLockFile( env, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD index b98dd56dd059a4..0ed4653e006aa8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD @@ -49,6 +49,7 @@ java_library( "//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/pkgcache", + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java index 882bde3f74bda6..657d6068c52f10 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java @@ -57,6 +57,9 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.pkgcache.PackageOptions; +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.runtime.BlazeCommand; import com.google.devtools.build.lib.runtime.BlazeCommandResult; import com.google.devtools.build.lib.runtime.Command; @@ -251,7 +254,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return reportAndCreateFailureResult( env, "Repositories not found: " + missingRepos, Code.INVALID_ARGUMENTS); } - try { + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.BZLMOD, "execute mod " + subcommand)) { dumpRepoMappings( repoMappingValues, new OutputStreamWriter( @@ -267,7 +271,10 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return reportAndCreateFailureResult( env, "the 'tidy' command doesn't take extra arguments", Code.TOO_MANY_ARGUMENTS); } - return runTidy(env, modTidyValue); + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.BZLMOD, "execute mod " + subcommand)) { + return runTidy(env, modTidyValue); + } } // Extract and check the --base_module argument first to use it when parsing the other args. @@ -524,7 +531,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe env.getReporter().getOutErr().getOutputStream(), modOptions.charset == UTF8 ? UTF_8 : US_ASCII)); - try { + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.BZLMOD, "execute mod " + subcommand)) { switch (subcommand) { case GRAPH -> modExecutor.graph(fromKeys); case DEPS -> modExecutor.graph(argsAsModules);