Skip to content

Commit

Permalink
Automated rollback of commit 0877340.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Increases build times in some circumstances:
see #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
  • Loading branch information
jmmv authored and copybara-github committed Mar 13, 2019
1 parent 4acf99a commit 3e660ad
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 50 deletions.
15 changes: 0 additions & 15 deletions src/main/cpp/blaze_util_darwin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
#include <sys/un.h>

#include <libproc.h>
#include <pthread/spawn.h>
#include <signal.h>
#include <spawn.h>
#include <stdlib.h>
#include <unistd.h>

Expand Down Expand Up @@ -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) {
}
Expand Down
6 changes: 0 additions & 6 deletions src/main/cpp/blaze_util_freebsd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include <limits.h>
#include <pwd.h>
#include <signal.h>
#include <spawn.h>
#include <string.h> // strerror
#include <sys/mount.h>
#include <sys/param.h>
Expand Down Expand Up @@ -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) {
}
Expand Down
6 changes: 0 additions & 6 deletions src/main/cpp/blaze_util_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <linux/magic.h>
#include <pwd.h>
#include <signal.h>
#include <spawn.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h> // strerror
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 1 addition & 23 deletions src/main/cpp/blaze_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,6 @@ void ExecuteProgram(const string& exe, const vector<string>& 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());
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -377,26 +366,15 @@ 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
<< ": " << GetLastErrorString();
}
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.
Expand Down

0 comments on commit 3e660ad

Please sign in to comment.