From 087734009801242b83655efb863b2d5a761ae3dc Mon Sep 17 00:00:00 2001 From: jmmv Date: Thu, 21 Feb 2019 13:10:47 -0800 Subject: [PATCH] Lower Bazel's QoS service class to Utility on macOS. Even though Bazel is an interactive tool, the operations it performs are not time-critical and should not starve system services (started by launchd, typically under the Utility class). To mitigate this problem, spawn the Bazel server under the Utility priority as well. In internal tests at Google, we have seen that this vastly improves build performance and overall system responsiveness when Bazel might compete with system services such as openvpn or FUSE daemons. Note that this is not a complete fix though. We are still letting the Bazel client run under the default class and the client still uses execv in a couple of places. First, in batch mode, and, second, in "bazel run". A possible fix for this would involve making the Bazel client respawn itself under the right priority at the very beginning but this has its own difficulties that should be looked into separately. Fixes https://github.com/bazelbuild/bazel/issues/7446 for the most part. RELNOTES: None. PiperOrigin-RevId: 235054167 --- src/main/cpp/blaze_util_darwin.cc | 15 +++++++++++++++ src/main/cpp/blaze_util_freebsd.cc | 6 ++++++ src/main/cpp/blaze_util_linux.cc | 6 ++++++ src/main/cpp/blaze_util_posix.cc | 24 +++++++++++++++++++++++- 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/main/cpp/blaze_util_darwin.cc b/src/main/cpp/blaze_util_darwin.cc index d20169c7642809..4bcc5b940f68a7 100644 --- a/src/main/cpp/blaze_util_darwin.cc +++ b/src/main/cpp/blaze_util_darwin.cc @@ -21,7 +21,9 @@ #include #include +#include #include +#include #include #include @@ -193,6 +195,19 @@ string GetSystemJavabase() { return javabase.substr(0, javabase.length()-1); } +int ConfigureDaemonProcess(posix_spawnattr_t* attrp) { + // The Bazel server and all of its subprocesses consume a ton of resources. + // + // It is common for these processes to rely on system services started by + // launchd and launchd-initiated services typically run as the Utility QoS + // class. We should run Bazel at the same level or otherwise we risk starving + // these services that we require to function properly. + // + // Explicitly lowering Bazel to run at the Utility QoS class also improves + // general system responsiveness. + return posix_spawnattr_set_qos_class_np(attrp, QOS_CLASS_UTILITY); +} + void WriteSystemSpecificProcessIdentifier( const string& server_dir, pid_t server_pid) { } diff --git a/src/main/cpp/blaze_util_freebsd.cc b/src/main/cpp/blaze_util_freebsd.cc index b9721688e7e590..6ae37a12c1959c 100644 --- a/src/main/cpp/blaze_util_freebsd.cc +++ b/src/main/cpp/blaze_util_freebsd.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include // strerror #include #include @@ -154,6 +155,11 @@ string GetSystemJavabase() { return "/usr/local/openjdk8"; } +int ConfigureDaemonProcess(posix_spawnattr_t* attrp) { + // No interesting platform-specific details to configure on this platform. + return 0; +} + void WriteSystemSpecificProcessIdentifier( const string& server_dir, pid_t server_pid) { } diff --git a/src/main/cpp/blaze_util_linux.cc b/src/main/cpp/blaze_util_linux.cc index 3801028e2ddb28..11047e2ec30828 100644 --- a/src/main/cpp/blaze_util_linux.cc +++ b/src/main/cpp/blaze_util_linux.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include // strerror @@ -212,6 +213,11 @@ static bool GetStartTime(const string& pid, string* start_time) { return true; } +int ConfigureDaemonProcess(posix_spawnattr_t* attrp) { + // No interesting platform-specific details to configure on this platform. + return 0; +} + void WriteSystemSpecificProcessIdentifier( const string& server_dir, pid_t server_pid) { string pid_string = ToString(server_pid); diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index c4206589c1d2b5..641878c0e24c84 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -276,6 +276,11 @@ void ExecuteProgram(const string& exe, const vector& args_vector) { BAZEL_LOG(INFO) << "Invoking binary " << exe << " in " << blaze_util::GetCwd(); + // TODO(jmmv): This execution does not respect any settings we might apply + // to the server process with ConfigureDaemonProcess when executed in the + // background as a daemon. Because we use that function to lower the + // priority of Bazel on macOS from a QoS perspective, this could have + // adverse scheduling effects on any tools invoked via ExecuteProgram. CharPP argv(args_vector); execv(exe.c_str(), argv.get()); } @@ -325,6 +330,12 @@ bool SocketBlazeServerStartup::IsStillAlive() { } } +// Sets platform-specific attributes for the creation of the daemon process. +// +// Returns zero on success or -1 on error, in which case errno is set to the +// corresponding error details. +int ConfigureDaemonProcess(posix_spawnattr_t* attrp); + void WriteSystemSpecificProcessIdentifier( const string& server_dir, pid_t server_pid); @@ -366,8 +377,18 @@ int ExecuteDaemon(const string& exe, << "Failed to modify posix_spawn_file_actions: "<< GetLastErrorString(); } + posix_spawnattr_t attrp; + if (posix_spawnattr_init(&attrp) == -1) { + BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) + << "Failed to create posix_spawnattr: "<< GetLastErrorString(); + } + if (ConfigureDaemonProcess(&attrp) == -1) { + BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) + << "Failed to modify posix_spawnattr: "<< GetLastErrorString(); + } + pid_t transient_pid; - if (posix_spawn(&transient_pid, daemonize.c_str(), &file_actions, NULL, + if (posix_spawn(&transient_pid, daemonize.c_str(), &file_actions, &attrp, CharPP(daemonize_args).get(), CharPP(env).get()) == -1) { BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) << "Failed to execute JVM via " << daemonize @@ -375,6 +396,7 @@ int ExecuteDaemon(const string& exe, } close(fds[1]); + posix_spawnattr_destroy(&attrp); posix_spawn_file_actions_destroy(&file_actions); // Wait for daemonize to exit. This guarantees that the pid file exists.