Skip to content

Commit

Permalink
Allow setting the Blaze server's QoS class via a --macos_qos_class flag.
Browse files Browse the repository at this point in the history
This adds a new flag to set the QoS class of the Bazel server and accepts
selecting any of the possible classes exposed by the system.  We will use
this flag to evaluate the behavior of Bazel under different classes because
it is not clear which one we should be using.  (We tried forcing the class
to be utility, but that caused regressions in some cases.)

This is essentially a retry of 0877340 but gated by a flag (which
is how it should have been done in the first place).

Note that the flag exists on all platforms to ensure that a bazelrc file
shared across different people works in all cases, but this flag is ignored
in non-macOS systems.

Addresses #7446.

RELNOTES: None.
PiperOrigin-RevId: 238440116
  • Loading branch information
jmmv authored and copybara-github committed Mar 14, 2019
1 parent a554d7c commit 3cb1aef
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 4 deletions.
3 changes: 2 additions & 1 deletion src/main/cpp/blaze.cc
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,8 @@ static int StartServer(const WorkspaceLayout *workspace_layout,

return ExecuteDaemon(exe, jvm_args_vector, PrepareEnvironmentForJvm(),
globals->jvm_log_file, globals->jvm_log_file_append,
binaries_dir, server_dir, server_startup);
binaries_dir, server_dir, globals->options,
server_startup);
}

// Replace this process with blaze in standalone/batch mode.
Expand Down
8 changes: 8 additions & 0 deletions src/main/cpp/blaze_util_darwin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
#include <sys/un.h>

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

Expand All @@ -32,6 +34,7 @@
#include <cstring>

#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/startup_options.h"
#include "src/main/cpp/util/errors.h"
#include "src/main/cpp/util/exit_code.h"
#include "src/main/cpp/util/file.h"
Expand Down Expand Up @@ -193,6 +196,11 @@ string GetSystemJavabase() {
return javabase.substr(0, javabase.length()-1);
}

int ConfigureDaemonProcess(posix_spawnattr_t *attrp,
const StartupOptions *options) {
return posix_spawnattr_set_qos_class_np(attrp, options->macos_qos_class);
}

void WriteSystemSpecificProcessIdentifier(
const string& server_dir, pid_t server_pid) {
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/cpp/blaze_util_freebsd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#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 @@ -154,6 +155,12 @@ string GetSystemJavabase() {
return "/usr/local/openjdk8";
}

int ConfigureDaemonProcess(posix_spawnattr_t *attrp,
const StartupOptions *options) {
// No interesting platform-specific details to configure on this platform.
return 0;
}

void WriteSystemSpecificProcessIdentifier(
const string& server_dir, pid_t server_pid) {
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/cpp/blaze_util_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#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 @@ -203,6 +204,12 @@ static bool GetStartTime(const string& pid, string* start_time) {
return true;
}

int ConfigureDaemonProcess(posix_spawnattr_t* attrp,
const StartupOptions* options) {
// 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
2 changes: 2 additions & 0 deletions src/main/cpp/blaze_util_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Dumper* Create(std::string* error = nullptr);
} // namespace embedded_binaries

struct GlobalVariables;
class StartupOptions;

class SignalHandler {
public:
Expand Down Expand Up @@ -157,6 +158,7 @@ int ExecuteDaemon(const std::string& exe,
const bool daemon_output_append,
const std::string& binaries_dir,
const std::string& server_dir,
const StartupOptions* options,
BlazeServerStartup** server_startup);

// A character used to separate paths in a list.
Expand Down
28 changes: 26 additions & 2 deletions src/main/cpp/blaze_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@ 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 @@ -325,6 +330,13 @@ 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,
const StartupOptions* options);

void WriteSystemSpecificProcessIdentifier(
const string& server_dir, pid_t server_pid);

Expand All @@ -335,6 +347,7 @@ int ExecuteDaemon(const string& exe,
const bool daemon_output_append,
const string& binaries_dir,
const string& server_dir,
const StartupOptions* options,
BlazeServerStartup** server_startup) {
const string pid_file = blaze_util::JoinPath(server_dir, kServerPidFile);
const string daemonize = blaze_util::JoinPath(binaries_dir, "daemonize");
Expand Down Expand Up @@ -366,15 +379,26 @@ 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, options) == -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,
CharPP(daemonize_args).get(), CharPP(env).get()) == -1) {
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
<< ": " << 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
2 changes: 1 addition & 1 deletion src/main/cpp/blaze_util_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -642,14 +642,14 @@ class ProcessHandleBlazeServerStartup : public BlazeServerStartup {
AutoHandle proc;
};


int ExecuteDaemon(const string& exe,
const std::vector<string>& args_vector,
const std::map<string, EnvVarValue>& env,
const string& daemon_output,
const bool daemon_out_append,
const string& binaries_dir,
const string& server_dir,
const StartupOptions* options,
BlazeServerStartup** server_startup) {
wstring wdaemon_output;
string error;
Expand Down
35 changes: 35 additions & 0 deletions src/main/cpp/startup_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <cstdio>
#include <cstdlib>
#include <cstring>

#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/blaze_util_platform.h"
Expand Down Expand Up @@ -96,6 +97,9 @@ StartupOptions::StartupOptions(const string &product_name,
digest_function(),
idle_server_tasks(true),
original_startup_options_(std::vector<RcStartupFlag>()),
#if defined(__APPLE__)
macos_qos_class(QOS_CLASS_DEFAULT),
#endif
unlimit_coredumps(false) {
if (blaze::IsRunningWithinTest()) {
output_root = blaze_util::MakeAbsolute(blaze::GetPathEnv("TEST_TMPDIR"));
Expand Down Expand Up @@ -151,6 +155,7 @@ StartupOptions::StartupOptions(const string &product_name,
RegisterUnaryStartupFlag("invocation_policy");
RegisterUnaryStartupFlag("io_nice_level");
RegisterUnaryStartupFlag("install_base");
RegisterUnaryStartupFlag("macos_qos_class");
RegisterUnaryStartupFlag("max_idle_secs");
RegisterUnaryStartupFlag("output_base");
RegisterUnaryStartupFlag("output_user_root");
Expand Down Expand Up @@ -290,6 +295,36 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
return blaze_exit_code::BAD_ARGV;
}
option_sources["max_idle_secs"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg, "--macos_qos_class")) !=
NULL) {
// We parse the value of this flag on all platforms even if it is
// macOS-specific to ensure that rc files mentioning it are valid.
if (strcmp(value, "user-interactive") == 0) {
#if defined(__APPLE__)
macos_qos_class = QOS_CLASS_USER_INTERACTIVE;
#endif
} else if (strcmp(value, "user-initiated") == 0) {
#if defined(__APPLE__)
macos_qos_class = QOS_CLASS_USER_INITIATED;
#endif
} else if (strcmp(value, "default") == 0) {
#if defined(__APPLE__)
macos_qos_class = QOS_CLASS_DEFAULT;
#endif
} else if (strcmp(value, "utility") == 0) {
#if defined(__APPLE__)
macos_qos_class = QOS_CLASS_UTILITY;
#endif
} else if (strcmp(value, "background") == 0) {
#if defined(__APPLE__)
macos_qos_class = QOS_CLASS_BACKGROUND;
#endif
} else {
blaze_util::StringPrintf(
error, "Invalid argument to --macos_qos_class: '%s'.", value);
return blaze_exit_code::BAD_ARGV;
}
option_sources["macos_qos_class"] = rcfile;
} else if (GetNullaryOption(arg, "--shutdown_on_low_sys_mem")) {
shutdown_on_low_sys_mem = true;
option_sources["shutdown_on_low_sys_mem"] = rcfile;
Expand Down
9 changes: 9 additions & 0 deletions src/main/cpp/startup_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
#ifndef BAZEL_SRC_MAIN_CPP_STARTUP_OPTIONS_H_
#define BAZEL_SRC_MAIN_CPP_STARTUP_OPTIONS_H_

#if defined(__APPLE__)
#include <sys/qos.h>
#endif

#include <map>
#include <memory>
#include <set>
Expand Down Expand Up @@ -310,6 +314,11 @@ class StartupOptions {
// Whether to raise the soft coredump limit to the hard one or not.
bool unlimit_coredumps;

#if defined(__APPLE__)
// The QoS class to apply to the Bazel server process.
qos_class_t macos_qos_class;
#endif

protected:
// Constructor for subclasses only so that site-specific extensions of this
// class can override the product name. The product_name must be the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,4 +483,18 @@ public String getTypeDescription() {
+ " Bash-style is buggy, the Windows-style is correct. See"
+ " https://github.com/bazelbuild/bazel/issues/7122")
public boolean windowsStyleArgEscaping;

@Option(
name = "macos_qos_class",
defaultValue = "default", // Only for documentation; value is set and used by the client.
documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
effectTags = {
OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS,
},
help =
"Sets the QoS service class of the %{product} server when running on macOS. This "
+ "flag has no effect on all other platforms but is supported to ensure rc files "
+ "can be shared among them without changes. Possible values are: user-interactive, "
+ "user-initiated, default, utility, and background.")
public String macosQosClass;
}
1 change: 1 addition & 0 deletions src/test/cpp/bazel_startup_options_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ TEST_F(BazelStartupOptionsTest, ValidStartupFlags) {
ExpectIsUnaryOption(options, "invocation_policy");
ExpectIsUnaryOption(options, "io_nice_level");
ExpectIsUnaryOption(options, "install_base");
ExpectIsUnaryOption(options, "macos_qos_class");
ExpectIsUnaryOption(options, "max_idle_secs");
ExpectIsUnaryOption(options, "output_base");
ExpectIsUnaryOption(options, "output_user_root");
Expand Down
16 changes: 16 additions & 0 deletions src/test/shell/integration/client_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,20 @@ function test_proxy_settings() {
fi
}

function test_macos_qos_class() {
for class in user-interactive user-initiated default utility background; do
bazel --macos_qos_class="${class}" info >"${TEST_log}" 2>&1 \
|| fail "Unknown QoS class ${class}"
# On macOS it'd be nice to verify that the server is indeed running at the
# desired class... but this is very hard to do. Common utilities do not
# print the QoS level, and powermetrics (which requires root privileges)
# only reports it under load -- so an "info" command is insufficient and the
# real thing would be quite expensive.
done

bazel --macos_qos_class=foo >"${TEST_log}" 2>&1 \
&& fail "Expected failure with invalid QoS class name"
expect_log "Invalid argument.*qos_class.*foo"
}

run_suite "Tests of the bazel client."

0 comments on commit 3cb1aef

Please sign in to comment.