Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow setting the priority of spawned subprocesses #2216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+123 −43
Open

Allow setting the priority of spawned subprocesses #2216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/build.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,8 @@ size_t RealCommandRunner::CanRunMore() const {

bool RealCommandRunner::StartCommand(Edge* edge) {
string command = edge->EvaluateCommand();
Subprocess* subproc = subprocs_.Add(command, edge->use_console());
Subprocess* subproc =
subprocs_.Add(command, config_.subprocess_priority, edge->use_console());
if (!subproc)
return false;
subproc_to_edge_.insert(make_pair(subproc, edge));
Expand Down
9 changes: 6 additions & 3 deletions src/build.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
#ifndef NINJA_BUILD_H_
#define NINJA_BUILD_H_

#include <climits>
#include <cstdio>
#include <map>
#include <memory>
#include <string>
#include <vector>

#include "depfile_parser.h"
#include "graph.h"
#include "exit_status.h"
#include "graph.h"
#include "util.h" // int64_t

struct BuildLog;
Expand Down Expand Up @@ -165,8 +166,9 @@ struct CommandRunner {

/// Options (e.g. verbosity, parallelism) passed to a build.
struct BuildConfig {
BuildConfig() : verbosity(NORMAL), dry_run(false), parallelism(1),
failures_allowed(1), max_load_average(-0.0f) {}
BuildConfig()
: verbosity(NORMAL), dry_run(false), parallelism(1), failures_allowed(1),
subprocess_priority(INT_MIN), max_load_average(-0.0f) {}

enum Verbosity {
QUIET, // No output -- used when testing.
Expand All @@ -178,6 +180,7 @@ struct BuildConfig {
bool dry_run;
int parallelism;
int failures_allowed;
int subprocess_priority;
/// The maximum load average we must not exceed. A negative value
/// means that we do not have any limit.
double max_load_average;
Expand Down
69 changes: 45 additions & 24 deletions src/ninja.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,29 +214,41 @@ struct Tool {

/// Print usage information.
void Usage(const BuildConfig& config) {
fprintf(stderr,
"usage: ninja [options] [targets...]\n"
"\n"
"if targets are unspecified, builds the 'default' target (see manual).\n"
"\n"
"options:\n"
" --version print ninja version (\"%s\")\n"
" -v, --verbose show all command lines while building\n"
" --quiet don't show progress status, just command output\n"
"\n"
" -C DIR change to DIR before doing anything else\n"
" -f FILE specify input build file [default=build.ninja]\n"
"\n"
" -j N run N jobs in parallel (0 means infinity) [default=%d on this system]\n"
" -k N keep going until N jobs fail (0 means infinity) [default=1]\n"
" -l N do not start new jobs if the load average is greater than N\n"
" -n dry run (don't run commands but act like they succeeded)\n"
"\n"
" -d MODE enable debugging (use '-d list' to list modes)\n"
" -t TOOL run a subtool (use '-t list' to list subtools)\n"
" terminates toplevel options; further flags are passed to the tool\n"
" -w FLAG adjust warnings (use '-w list' to list warnings)\n",
kNinjaVersion, config.parallelism);
fprintf(
stderr,
"usage: ninja [options] [targets...]\n"
"\n"
"if targets are unspecified, builds the 'default' target (see manual).\n"
"\n"
"options:\n"
" --version print ninja version (\"%s\")\n"
" -v, --verbose show all command lines while building\n"
" --quiet don't show progress status, just command output\n"
"\n"
" -C DIR change to DIR before doing anything else\n"
" -f FILE specify input build file [default=build.ninja]\n"
"\n"
" -j N run N jobs in parallel (0 means infinity) [default=%d "
"on this system]\n"
" -k N keep going until N jobs fail (0 means infinity) "
"[default=1]\n"
" -l N do not start new jobs if the load average is greater "
"than N\n"
" -n dry run (don't run commands but act like they "
"succeeded)\n"
" -p PRIORITY set the priority/niceness of spawned processes\n"
" the range of possible values is platform dependent:\n"
" - POSIX: In -20..19 range, and -20..20 on some "
"systems, see 'man setpriority'n"
" - Windows: between -20 (highest priority) and 20 "
"(lowest priority)\n"
"\n"
" -d MODE enable debugging (use '-d list' to list modes)\n"
" -t TOOL run a subtool (use '-t list' to list subtools)\n"
" terminates toplevel options; further flags are passed "
"to the tool\n"
" -w FLAG adjust warnings (use '-w list' to list warnings)\n",
kNinjaVersion, config.parallelism);
}

/// Choose a default value for the -j (parallelism) flag.
Expand Down Expand Up @@ -1447,7 +1459,7 @@ int ReadFlags(int* argc, char*** argv,

int opt;
while (!options->tool &&
(opt = getopt_long(*argc, *argv, "d:f:j:k:l:nt:vw:C:h", kLongOptions,
(opt = getopt_long(*argc, *argv, "d:f:j:k:l:p:nt:vw:C:h", kLongOptions,
NULL)) != -1) {
switch (opt) {
case 'd':
Expand Down Expand Up @@ -1492,6 +1504,15 @@ int ReadFlags(int* argc, char*** argv,
case 'n':
config->dry_run = true;
break;
case 'p': {
char* end;
int value = strtol(optarg, &end, 10);
if (*end != 0)
Fatal("-p parameter not numeric: did you mean -p 0?");

config->subprocess_priority = value;
break;
}
case 't':
options->tool = ChooseTool(optarg);
if (!options->tool)
Expand Down
28 changes: 20 additions & 8 deletions src/subprocess-posix.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,20 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "subprocess.h"

#include <sys/select.h>
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
#include <limits.h>
#include <spawn.h>
#include <stdio.h>
#include <string.h>
#include <sys/resource.h>
#include <sys/select.h>
#include <sys/time.h>
#include <sys/wait.h>
#include <spawn.h>
#include <unistd.h>

#include "subprocess.h"

#if defined(USE_PPOLL)
#include <poll.h>
Expand All @@ -48,7 +51,8 @@ Subprocess::~Subprocess() {
Finish();
}

bool Subprocess::Start(SubprocessSet* set, const string& command) {
bool Subprocess::Start(SubprocessSet* set, const string& command,
int priority) {
int output_pipe[2];
if (pipe(output_pipe) < 0)
Fatal("pipe: %s", strerror(errno));
Expand Down Expand Up @@ -123,6 +127,13 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
if (err != 0)
Fatal("posix_spawn: %s", strerror(err));

if (priority != INT_MIN) {
err = setpriority(PRIO_PROCESS, pid_, priority);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a race condition where the spawned process may have already completed before this code is run. The call would return an error and this would crash Ninja unexpectedly.

Can you try to set the priority before the posix_spawn() call with posix_spawnattr_setschedparam() instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow, using posix_spawnattr_setschedparam doesn't seem to work. When checking in htop I don't see any change in niceness of the spawned processes.

if (err != 0 && err != ESRCH) {
Fatal("setpriority: %s", strerror(errno));
}
}

err = posix_spawnattr_destroy(&attr);
if (err != 0)
Fatal("posix_spawnattr_destroy: %s", strerror(err));
Expand Down Expand Up @@ -238,9 +249,10 @@ SubprocessSet::~SubprocessSet() {
Fatal("sigprocmask: %s", strerror(errno));
}

Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
Subprocess* SubprocessSet::Add(const string& command, int priority,
bool use_console) {
Subprocess *subprocess = new Subprocess(use_console);
if (!subprocess->Start(this, command)) {
if (!subprocess->Start(this, command, priority)) {
delete subprocess;
return 0;
}
Expand Down
24 changes: 21 additions & 3 deletions src/subprocess-win32.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,22 @@ HANDLE Subprocess::SetupPipe(HANDLE ioport) {
return output_write_child;
}

bool Subprocess::Start(SubprocessSet* set, const string& command) {
static DWORD POSIXPriorityToWin32(int unixPriority) {
if (unixPriority <= -20) {
return HIGH_PRIORITY_CLASS; // ..-20
} else if (unixPriority < 0) {
return ABOVE_NORMAL_PRIORITY_CLASS; // -19..-1
} else if (unixPriority == 0) {
return NORMAL_PRIORITY_CLASS; // 0
} else if (unixPriority < 20) {
return BELOW_NORMAL_PRIORITY_CLASS; // 1..19
} else {
return IDLE_PRIORITY_CLASS; // 20..
}
clemenswasser marked this conversation as resolved.
Show resolved Hide resolved
}

bool Subprocess::Start(SubprocessSet* set, const string& command,
int priority) {
HANDLE child_pipe = SetupPipe(set->ioport_);

SECURITY_ATTRIBUTES security_attributes;
Expand Down Expand Up @@ -106,6 +121,8 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {

// Ninja handles ctrl-c, except for subprocesses in console pools.
DWORD process_flags = use_console_ ? 0 : CREATE_NEW_PROCESS_GROUP;
if (priority != INT_MIN)
process_flags |= POSIXPriorityToWin32(priority);

// Do not prepend 'cmd /c' on Windows, this breaks command
// lines greater than 8,191 chars.
Expand Down Expand Up @@ -238,9 +255,10 @@ BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) {
return FALSE;
}

Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
Subprocess* SubprocessSet::Add(const string& command, int priority,
bool use_console) {
Subprocess *subprocess = new Subprocess(use_console);
if (!subprocess->Start(this, command)) {
if (!subprocess->Start(this, command, priority)) {
delete subprocess;
return 0;
}
Expand Down
16 changes: 13 additions & 3 deletions src/subprocess.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
#ifndef NINJA_SUBPROCESS_H_
#define NINJA_SUBPROCESS_H_

#include <limits.h>

#include <queue>
#include <string>
#include <vector>
#include <queue>

#ifdef _WIN32
#include <windows.h>
Expand Down Expand Up @@ -52,7 +54,13 @@ struct Subprocess {

private:
Subprocess(bool use_console);
bool Start(struct SubprocessSet* set, const std::string& command);
// priority:
// - INT_MIN denotes no intended priority change
// - The range of possible values is platform dependent
// - POSIX: typically between -20 and 19 but can be changed by RLIMIT_NICE
// - Windows: between -20 (highest priority) and 20 (lowest priority)
bool Start(struct SubprocessSet* set, const std::string& command,
int priority);
void OnPipeReady();

std::string buf_;
Expand Down Expand Up @@ -83,7 +91,9 @@ struct SubprocessSet {
SubprocessSet();
~SubprocessSet();

Subprocess* Add(const std::string& command, bool use_console = false);
// priority is forwarded to Subprocess::Start
Subprocess* Add(const std::string& command, int priority = INT_MIN,
bool use_console = false);
bool DoWork();
Subprocess* NextFinished();
void Clear();
Expand Down
17 changes: 16 additions & 1 deletion src/subprocess_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ TEST_F(SubprocessTest, Console) {
// Skip test if we don't have the console ourselves.
if (isatty(0) && isatty(1) && isatty(2)) {
Subprocess* subproc =
subprocs_.Add("test -t 0 -a -t 1 -a -t 2", /*use_console=*/true);
subprocs_.Add("test -t 0 -a -t 1 -a -t 2", /*priority=*/INT_MIN,
/*use_console=*/true);
ASSERT_NE((Subprocess*)0, subproc);

while (!subproc->Done()) {
Expand All @@ -161,6 +162,20 @@ TEST_F(SubprocessTest, Console) {
}
}

TEST_F(SubprocessTest, Priority) {
// Field 19 of the stat file is the nice level according to:
// https://www.kernel.org/doc/html/latest/filesystems/proc.html
Subprocess* subproc =
subprocs_.Add("cat /proc/self/stat | cut -d' ' -f19", /*priority=*/10);
ASSERT_NE((Subprocess*)0, subproc);

while (!subproc->Done()) {
subprocs_.DoWork();
}

EXPECT_EQ("10\n", subproc->GetOutput());
}

#endif

TEST_F(SubprocessTest, SetWithSingle) {
Expand Down
Loading