bazel: make Starlark profiler work in Bazel
Fixes https://github.com/bazelbuild/bazel/issues/10890
Details:
- configure copybara so that Bazel sees JNI.java.oss as JNI.java.
- use the correct .so library name for Bazel (no "-jni").
- add the dynamic library to to Bazel's install-base using
the platform's required name (libx.so, libx.dylib, x.dll).
- report errors during command startup, don't just store them. :)
- don't consider SIG_DFL as "signal already in use". The pthreads
package used by Bazel sets SIGPROF to DFL whereas Blaze's uses IGN.
RELNOTES:
The --starlark_cpu_profile=<file> flag writes a profile in
pprof format containing a statistical summary of CPU usage
by all Starlark execution during the bazel command. Use it
to identify slow Starlark functions in loading and analysis.
PiperOrigin-RevId: 315678587
diff --git a/src/BUILD b/src/BUILD
index 1b03374..0cb189a 100644
--- a/src/BUILD
+++ b/src/BUILD
@@ -12,6 +12,9 @@
name = "install_base_key-file" + suffix,
srcs = [
"//src/main/java/com/google/devtools/build/lib/bazel:BazelServer_deploy.jar",
+ "//src/main/java/com/google/devtools/build/lib/syntax:cpu_profiler",
+ # TODO(brandjon): ensure we haven't forgotten any package-zip items,
+ # otherwise bazel won't correctly reextract modified files.
"//src/main/cpp:client",
"//src/main/tools:build-runfiles",
"//src/main/tools:process-wrapper",
@@ -329,6 +332,8 @@
"//src/main/java/com/google/devtools/build/lib/bazel:BazelServer_deploy.jar",
"install_base_key" + suffix,
":platforms_archive",
+ # Ordinary items follow:
+ "//src/main/java/com/google/devtools/build/lib/syntax:cpu_profiler",
"//src/main/native:target-os-unix-native-lib",
"//src/main/native/windows:target-os-native-lib",
"//src/main/tools:build-runfiles",
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 2d4d6b1..0e67901 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -349,7 +349,7 @@
out = new FileOutputStream(commonOptions.starlarkCpuProfile);
} catch (IOException ex) {
String message = "Starlark CPU profiler: " + ex.getMessage();
- storedEventHandler.handle(Event.error(message));
+ outErr.printErrLn(message);
return createDetailedCommandResult(
message,
ExitCode.LOCAL_ENVIRONMENTAL_ERROR,
@@ -359,7 +359,7 @@
Starlark.startCpuProfile(out, Duration.ofMillis(10));
} catch (IllegalStateException ex) { // e.g. SIGPROF in use
String message = Strings.nullToEmpty(ex.getMessage());
- storedEventHandler.handle(Event.error(message));
+ outErr.printErrLn(message);
return createDetailedCommandResult(
message,
ExitCode.LOCAL_ENVIRONMENTAL_ERROR,
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BUILD b/src/main/java/com/google/devtools/build/lib/syntax/BUILD
index cc7c91b..0088f6b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BUILD
@@ -122,7 +122,6 @@
"StringModule.java",
"Tuple.java",
],
- data = [":libcpu_profiler.so"], # will be loaded dynamically
deps = [
":frontend",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
@@ -135,6 +134,35 @@
],
)
+# Dynamic library for Starlark CPU profiler.
+# The application or test must arrange for the library to be added
+# to the java.library.path directory (e.g. by 'java -Djava.library.path=<dir>').
+# What a mess.
+filegroup(
+ name = "cpu_profiler",
+ srcs = select({
+ "//src/conditions:darwin": [":libcpu_profiler.dylib"],
+ "//src/conditions:windows": [":cpu_profiler.dll"],
+ "//conditions:default": [":libcpu_profiler.so"], # POSIX
+ }),
+)
+
+genrule(
+ name = "cpu_profiler_darwin",
+ srcs = ["libcpu_profiler.so"],
+ outs = ["libcpu_profiler.dylib"],
+ cmd = "cp $< $@",
+ visibility = ["//visibility:private"],
+)
+
+genrule(
+ name = "cpu_profiler_windows",
+ srcs = ["libcpu_profiler.so"],
+ outs = ["cpu_profiler.dll"],
+ cmd = "cp $< $@",
+ visibility = ["//visibility:private"],
+)
+
# The C++ portion of the Starlark CPU profiler.
cc_binary(
name = "libcpu_profiler.so",
@@ -150,5 +178,6 @@
"//conditions:default": ["cpu_profiler_unimpl.cc"],
}),
linkshared = 1,
+ visibility = ["//visibility:private"],
deps = ["@bazel_tools//tools/jdk:jni"],
)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/JNI.java b/src/main/java/com/google/devtools/build/lib/syntax/JNI.java
index b3d514f..b8de3009 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/JNI.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/JNI.java
@@ -18,8 +18,13 @@
private JNI() {} // uninstantiable
static void load() {
- // Google's Java launcher statically links the JNI
- // dependencies of a java_binary at build time,
- // so there is nothing to do here.
+ try {
+ System.loadLibrary("cpu_profiler");
+ } catch (UnsatisfiedLinkError ex) {
+ // Ignore, deferring the error until a C function is called, if ever.
+ // Without this hack //src/test/shell/bazel:bazel_bootstrap_distfile_test
+ // fails with an utterly uninformative error.
+ // TODO(adonovan): remove try/catch once that test is fixed.
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/JNI.java.oss b/src/main/java/com/google/devtools/build/lib/syntax/JNI.java.oss
deleted file mode 100644
index 0e5bfee..0000000
--- a/src/main/java/com/google/devtools/build/lib/syntax/JNI.java.oss
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright 2020 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.syntax;
-
-final class JNI {
- private JNI() {} // uninstantiable
-
- static void load() {
- System.loadLibrary("cpu_profiler-jni");
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/cpu_profiler_posix.cc b/src/main/java/com/google/devtools/build/lib/syntax/cpu_profiler_posix.cc
index 1d42662..c64b30d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/cpu_profiler_posix.cc
+++ b/src/main/java/com/google/devtools/build/lib/syntax/cpu_profiler_posix.cc
@@ -157,9 +157,11 @@
// Is a handler already in effect?
// Check for 3-arg and 1-arg forms.
typedef void (*sighandler_t)(int); // don't rely on this GNU extension
- if ((oldact.sa_flags & SA_SIGINFO) != 0
- ? (reinterpret_cast<sighandler_t>(oldact.sa_sigaction) != SIG_IGN)
- : (oldact.sa_handler != SIG_IGN)) {
+ sighandler_t prev = (oldact.sa_flags & SA_SIGINFO) != 0
+ ? reinterpret_cast<sighandler_t>(oldact.sa_sigaction)
+ : oldact.sa_handler;
+ // The initial handler (DFL or IGN) may vary by thread package.
+ if (prev != SIG_DFL && prev != SIG_IGN) {
// Someone else is profiling this JVM.
// Restore their handler and fail.
(void)sigaction(SIGPROF, &oldact, nullptr);
diff --git a/src/test/shell/integration/loading_phase_test.sh b/src/test/shell/integration/loading_phase_test.sh
index b4a9d1b..f9fd9e7 100755
--- a/src/test/shell/integration/loading_phase_test.sh
+++ b/src/test/shell/integration/loading_phase_test.sh
@@ -472,4 +472,30 @@
expect_not_log "//foo_prefix"
}
+function test_starlark_cpu_profile() {
+ if $is_windows; then
+ echo "Starlark profiler is not supported on Microsoft Windows."
+ return
+ fi
+
+ mkdir -p test
+ echo 'load("inc.bzl", "main"); main()' > test/BUILD
+ cat >> test/inc.bzl <<'EOF'
+def main():
+ for i in range(2000):
+ foo()
+def foo():
+ list(range(10000))
+ sorted(range(10000))
+main() # uses ~3 seconds of CPU
+EOF
+ bazel query --starlark_cpu_profile="${TEST_TMPDIR}/pprof.gz" test/BUILD
+ # We don't depend on pprof, so just look for some strings in the raw file.
+ gunzip "${TEST_TMPDIR}/pprof.gz"
+ for str in sorted list range foo test/BUILD test/inc.bzl main; do
+ grep -q sorted "${TEST_TMPDIR}/pprof" ||
+ fail "string '$str' not found in profiler output"
+ done
+}
+
run_suite "Integration tests of ${PRODUCT_NAME} using loading/analysis phases."
