From 3e660ad178926648e8e10e2ee7a1a30b12f9b3d1 Mon Sep 17 00:00:00 2001 From: jmmv Date: Wed, 13 Mar 2019 07:00:25 -0700 Subject: [PATCH] Automated rollback of commit 087734009801242b83655efb863b2d5a761ae3dc. *** Reason for rollback *** Increases build times in some circumstances: see https://github.com/bazelbuild/bazel/issues/7682. I haven't yet investigated why this is great in some cases and bad in others, so undoing this for now is best. *** Original change description *** 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... *** RELNOTES: None. PiperOrigin-RevId: 238217783 --- 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, 1 insertion(+), 50 deletions(-) diff --git a/src/main/cpp/blaze_util_darwin.cc b/src/main/cpp/blaze_util_darwin.cc index 4bcc5b940f68a7..d20169c7642809 100644 --- a/src/main/cpp/blaze_util_darwin.cc +++ b/src/main/cpp/blaze_util_darwin.cc @@ -21,9 +21,7 @@ #include #include -#include #include -#include #include #include @@ -195,19 +193,6 @@ 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 6ae37a12c1959c..b9721688e7e590 100644 --- a/src/main/cpp/blaze_util_freebsd.cc +++ b/src/main/cpp/blaze_util_freebsd.cc @@ -16,7 +16,6 @@ #include #include #include -#include #include // strerror #include #include @@ -155,11 +154,6 @@ 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 cd12524033bd35..a365ac91c47dec 100644 --- a/src/main/cpp/blaze_util_linux.cc +++ b/src/main/cpp/blaze_util_linux.cc @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include // strerror @@ -204,11 +203,6 @@ 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 641878c0e24c84..c4206589c1d2b5 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -276,11 +276,6 @@ 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()); } @@ -330,12 +325,6 @@ 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); @@ -377,18 +366,8 @@ 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, &attrp, + if (posix_spawn(&transient_pid, daemonize.c_str(), &file_actions, NULL, CharPP(daemonize_args).get(), CharPP(env).get()) == -1) { BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) << "Failed to execute JVM via " << daemonize @@ -396,7 +375,6 @@ 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.