From 1a504f100f29b9ab52bf75dbad81d83d74e50851 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 12 Feb 2024 01:50:47 -0800 Subject: [PATCH] [7.1.0] Add support for additional command profiler event types. Also replace the existing Java test with a shell test; the former currently fails with an obscure JVM crash, and it's unclear how to fix it. Note that it's not possible to simultaneously capture multiple event types. PiperOrigin-RevId: 606177945 Change-Id: Id6fbc68c65c1d46092c7a041ce81e1e2ff7f99db --- .../lib/profiler/CommandProfilerModule.java | 73 +++++++++++++----- .../google/devtools/build/lib/profiler/BUILD | 22 +----- .../profiler/CommandProfilerModuleTest.java | 66 ---------------- src/test/shell/bazel/BUILD | 8 ++ src/test/shell/bazel/command_profiler_test.sh | 76 +++++++++++++++++++ 5 files changed, 139 insertions(+), 106 deletions(-) delete mode 100644 src/test/java/com/google/devtools/build/lib/profiler/CommandProfilerModuleTest.java create mode 100755 src/test/shell/bazel/command_profiler_test.sh diff --git a/src/main/java/com/google/devtools/build/lib/profiler/CommandProfilerModule.java b/src/main/java/com/google/devtools/build/lib/profiler/CommandProfilerModule.java index 666a27bbfa0b17..9aa2a9ea3aacfa 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/CommandProfilerModule.java +++ b/src/main/java/com/google/devtools/build/lib/profiler/CommandProfilerModule.java @@ -21,33 +21,61 @@ import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionsBase; import java.time.Duration; +import java.util.Locale; import javax.annotation.Nullable; import one.profiler.AsyncProfiler; -/** Bazel module to record pprof-compatible profiles for single invocations. */ +/** Bazel module to record a Java Flight Recorder profile for a single command. */ public class CommandProfilerModule extends BlazeModule { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private static final Duration PROFILING_INTERVAL = Duration.ofMillis(10); + /** The type of profile to capture. */ + enum ProfileType { + CPU, + WALL, + ALLOC, + LOCK; + + @Override + public String toString() { + return name().toLowerCase(Locale.US); + } + } + + /** Options converter for --experimental_command_profile. */ + public static final class ProfileTypeConverter extends EnumConverter { + + public ProfileTypeConverter() { + super(ProfileType.class, "--experimental_command_profile setting"); + } + } + /** CommandProfilerModule options. */ public static final class Options extends OptionsBase { + @Option( name = "experimental_command_profile", - defaultValue = "false", + defaultValue = "null", + converter = ProfileTypeConverter.class, documentationCategory = OptionDocumentationCategory.LOGGING, effectTags = {OptionEffectTag.UNKNOWN}, help = - "Records a Java Flight Recorder CPU profile into a profile.jfr file in the output base" - + " directory. The syntax and semantics of this flag might change in the future to" - + " support different profile types or output formats; use at your own risk.") - public boolean captureCommandProfile; + "Records a Java Flight Recorder profile for the duration of the command. One of the" + + " supported profiling event types (cpu, wall, alloc or lock) must be given as an" + + " argument. The profile is written to a file named after the event type under the" + + " output base directory." + + " The syntax and semantics of this flag might change in the future to support" + + " additional profile types or output formats; use at your own risk.") + public ProfileType profileType; } @Override @@ -55,7 +83,7 @@ public Iterable> getCommandOptions(Command command) return ImmutableList.of(Options.class); } - private boolean captureCommandProfile; + @Nullable private ProfileType profileType; @Nullable private Reporter reporter; @Nullable private Path outputBase; @Nullable private Path outputPath; @@ -63,11 +91,11 @@ public Iterable> getCommandOptions(Command command) @Override public void beforeCommand(CommandEnvironment env) { Options options = env.getOptions().getOptions(Options.class); - captureCommandProfile = options.captureCommandProfile; + profileType = options.profileType; outputBase = env.getBlazeWorkspace().getOutputBase(); reporter = env.getReporter(); - if (!captureCommandProfile) { + if (profileType == null) { // Early exit so we don't attempt to load the JNI unless necessary. return; } @@ -77,24 +105,26 @@ public void beforeCommand(CommandEnvironment env) { return; } - outputPath = outputBase.getRelative("profile.jfr"); + outputPath = getProfilerOutputPath(profileType); try { - profiler.execute(getProfilerCommand(outputPath)); + profiler.execute(getProfilerCommand(profileType, outputPath)); } catch (Exception e) { // This may occur if the user has insufficient privileges to capture performance events. - reporter.handle(Event.error("Starting JFR CPU profile failed: " + e)); - captureCommandProfile = false; + reporter.handle( + Event.error(String.format("Starting JFR %s profile failed: %s", profileType, e))); + profileType = null; } - if (captureCommandProfile) { - reporter.handle(Event.info("Writing JFR CPU profile to " + outputPath)); + if (profileType != null) { + reporter.handle( + Event.info(String.format("Writing JFR %s profile to %s", profileType, outputPath))); } } @Override public void afterCommand() { - if (!captureCommandProfile) { + if (profileType == null) { // Early exit so we don't attempt to load the JNI unless necessary. return; } @@ -106,7 +136,7 @@ public void afterCommand() { profiler.stop(); - captureCommandProfile = false; + profileType = null; outputBase = null; reporter = null; outputPath = null; @@ -123,9 +153,14 @@ private static AsyncProfiler getProfiler() { return null; } - private static String getProfilerCommand(Path outputPath) { + private Path getProfilerOutputPath(ProfileType profileType) { + return outputBase.getChild(profileType + ".jfr"); + } + + private static String getProfilerCommand(ProfileType profileType, Path outputPath) { // See https://github.com/async-profiler/async-profiler/blob/master/src/arguments.cpp. return String.format( - "start,event=cpu,interval=%s,file=%s,jfr", PROFILING_INTERVAL.toNanos(), outputPath); + "start,event=%s,interval=%s,file=%s,jfr", + profileType, PROFILING_INTERVAL.toNanos(), outputPath); } } diff --git a/src/test/java/com/google/devtools/build/lib/profiler/BUILD b/src/test/java/com/google/devtools/build/lib/profiler/BUILD index fe88fcb412e344..58543de026cc1a 100644 --- a/src/test/java/com/google/devtools/build/lib/profiler/BUILD +++ b/src/test/java/com/google/devtools/build/lib/profiler/BUILD @@ -17,10 +17,7 @@ filegroup( java_test( name = "ProfilerTests", - srcs = glob( - ["*.java"], - exclude = ["CommandProfilerModuleTest.java"], - ), + srcs = glob(["*.java"]), test_class = "com.google.devtools.build.lib.AllTests", runtime_deps = [ "//src/test/java/com/google/devtools/build/lib:test_runner", @@ -42,20 +39,3 @@ java_test( "//third_party:truth", ], ) - -java_test( - name = "CommandProfilerModuleTest", - srcs = ["CommandProfilerModuleTest.java"], - tags = [ - # Bazel-specific tests - "manual", - ], - deps = [ - "//src/main/java/com/google/devtools/build/lib:runtime", - "//src/main/java/com/google/devtools/build/lib/profiler:command_profiler_module", - "//src/main/java/com/google/devtools/build/lib/util:os", - "//src/test/java/com/google/devtools/build/lib/buildtool/util", - "//third_party:junit4", - "//third_party:truth", - ], -) diff --git a/src/test/java/com/google/devtools/build/lib/profiler/CommandProfilerModuleTest.java b/src/test/java/com/google/devtools/build/lib/profiler/CommandProfilerModuleTest.java deleted file mode 100644 index 045e8d5d5e8beb..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/profiler/CommandProfilerModuleTest.java +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.profiler; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assume.assumeTrue; - -import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; -import com.google.devtools.build.lib.runtime.BlazeRuntime; -import com.google.devtools.build.lib.util.OS; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for {@link CommandProfilerModule}. */ -@RunWith(JUnit4.class) -public final class CommandProfilerModuleTest extends BuildIntegrationTestCase { - - @Override - protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { - return super.getRuntimeBuilder().addBlazeModule(new CommandProfilerModule()); - } - - @Before - public void setUp() throws Exception { - write("BUILD", ""); - } - - @Test - public void testProfilingDisabled() throws Exception { - buildTarget("//:BUILD"); - assertThat(outputBase.getChild("profile.jfr").exists()).isFalse(); - } - - @Test - public void testProfilingEnabled() throws Exception { - addOptions("--experimental_command_profile"); - - try { - buildTarget("//:BUILD"); - } catch (Exception e) { - // Linux perf events are not supported on CI. - // See https://github.com/async-profiler/async-profiler/#troubleshooting. - if (e.getMessage().contains("No access to perf events") - || e.getMessage().contains("Perf events unavailable")) { - return; - } - } - - assumeTrue(OS.getCurrent() == OS.LINUX || OS.getCurrent() == OS.DARWIN); - - assertThat(outputBase.getChild("profile.jfr").exists()).isTrue(); - } -} diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 9d0c4e92d81442..b406624c28c38b 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -1079,6 +1079,14 @@ sh_test( tags = ["no_windows"], ) +sh_test( + name = "command_profiler_test", + size = "medium", + srcs = ["command_profiler_test.sh"], + data = [":test-deps"], + tags = ["no_windows"], +) + sh_test( name = "execroot_test", size = "medium", diff --git a/src/test/shell/bazel/command_profiler_test.sh b/src/test/shell/bazel/command_profiler_test.sh new file mode 100755 index 00000000000000..44b0e63088dedd --- /dev/null +++ b/src/test/shell/bazel/command_profiler_test.sh @@ -0,0 +1,76 @@ +#!/bin/bash +# +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eu + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + + +function test_profiler_disabled() { + cat > BUILD <<'EOF' +genrule( + name = "gen", + outs = ["out"], + cmd = "touch $@", +) +EOF + + bazel build //:gen || fail "Expected build to succeed" + + if [[ "$(ls "$(bazel info output_base)/*.jfr")" ]]; then + fail "Expected no profiler outputs" + fi +} + +function do_test_profiler_enabled() { + local -r type="$1" + + cat > BUILD <<'EOF' +genrule( + name = "gen", + outs = ["out"], + cmd = "touch $@", +) +EOF + + bazel build --experimental_command_profile="${type}" //:gen \ + || fail "Expected build to succeed" + + if ! [[ -f "$(bazel info output_base)/${type}.jfr" ]]; then + fail "Expected profiler output" + fi +} + +function test_cpu_profiler_enabled() { + do_test_profiler_enabled cpu +} + +function test_wall_profiler_enabled() { + do_test_profiler_enabled wall +} + +function test_alloc_profiler_enabled() { + do_test_profiler_enabled alloc +} + +function test_lock_profiler_enabled() { + do_test_profiler_enabled lock +} + +run_suite "command profiler tests"