Environment variable for limiting the number of JOBS [Again]

[kobalicek]

Is it possible to finally add the support for an environment variable that would limit the maximum number of jobs ninja can spawn?

I would suggest having an optional environment variable NINJA_NUM_JOBS, which would be used as an initial value when provided. This variable will be parsed before command line arguments that could of course override it.

I'm not asking for "additional flags" support [https://github.com//pull/1399], just a way to override the initial guess of the number of jobs as I understand the concerns behind NINJA_FLAGS and I understand that it might never get accepted.

I'm proposing a small change like this:

int GuessParallelism() {
  int n = GetProcessorCount();
  switch (n) {
    case 0: 
    case 1: n = 2; break;
    case 2: n = 3; break;
    default: n += 2; break;
  }

  const char* s = getenv("NINJA_NUM_JOBS"); // or any better name.
  if (!s)
    return n;

  char* end;
  long nUserDefined = strtol(s, &end, 10);

  // If the environment value is invalid or is greater than our initial guess, drop it.
  if (*end != 0 || nUserDefined <= 0 || nUserDefined > n)
    return n;

  // Otherwise use the sane value provided by the user.
  return int(nUserDefined);
}

[jhasse]

Have you thought about writing a wrapper script which you put on your PATH named ninja which calls the real ninja with the amount of jobs you want?

[kobalicek]

"Anyone can make the simple complicated."

BTW all of these were already discussed in other issues [I read them]. I really don't understand why there is such refusal regarding this feature as it's just a useful one. Can I workaround it? Yeah, but is it a solution? No.

[jhasse]

Yeah, but is it a solution? No.

Why not?

[kobalicek]

You cannot see a difference between having a possibility to override a default value and between passing a default value to every invocation of ninja?

Please stop discussing something that was already discussed in other issues, I didn't create this issue to discuss workarounds. I'm wondering why such refusal and whether it has some reason.

[jhasse]

You cannot see a difference between having a possibility to override a default value and between passing a default value to every invocation of ninja?

I can see the difference.

I'm wondering why such refusal and whether it has some reason.

The reason for me would be that this can already be solved by creating a wrapper script, so there's no need to implement this in Ninja itself.

[kobalicek]

And this is what I'm trying to point out - wrapper script is not the same as environment variable and it's definitely not a solution. With your reasoning I would propose to remove support for NINJA_STATUS environment variable, because you can create a wrapper script, thus there is no point of having that in Ninja itself.

[kobalicek]

Please see these:

• Environment variable controlling max number of jobs #922
• Added support for environment variable NINJA_PARALLEL_JOBS #1363
• Added support for environment variable NINJA_FLAGS #1399

Possible workarounds were already discussed with some answers as well.

[jhasse]

With your reasoning I would propose to remove support for NINJA_STATUS environment variable, because you can create a wrapper script, thus there is no point of having that in Ninja itself.

A wrapper script can not replace all of the functionality that NINJA_STATUS provides, so there's a point of having it in Ninja itself.

Please see these

Thanks, I'll have a look.

[kobalicek]

I think the problem is how we see it. I see environment variable as an initial value when it's present, and "-j XXX" option as an override of that value. A script would always override that value and in such case I would have to maintain it and make sure that "each" process that executes ninja executes that script instead (not even sure this would be possible on Windows).

What I mean, we can discuss workarounds to no end, but it would not solve the real problem which is controlling the number of jobs through an environment variable. Environment variable is easy to add and modify and it's much better way than creating a wrapper scripts. I can leave it on machines on which I don't care and add it on machine where I'm having problems with the initial Ninja guess.

And considering this is not a drastic change I don't see a reason "why not". I mean the reason I proposed environment variable to only control the number of jobs was that I didn't want to go with a more general NINJA_FLAGS, which could mean a more complicated logic. So I'm really not trying to complicate it, I think having support for one more environment variable would not hurt Ninja and it would actually help some people.

[jhasse]

A script would always override that value

No, you can adjust the script so that it doesn't always overwrite the value.

make sure that "each" process that executes ninja executes that script instead (not even sure this would be possible on Windows).

Yes, this is possible on Windows. The PATH works nearly identical there, so you just have to put your script in a directory that comes before the real ninja executable (or just remove the real ninja executable from the PATH).

What I mean, we can discuss workarounds to no end, but it would not solve the real problem which is controlling the number of jobs through an environment variable.

But it would, as you can read in the environment variable in your script.

And considering this is not a drastic change I don't see a reason "why not".

nico explained some of the "why not" arguments in #1399.

[kobalicek]

It was explained why not NINJA_FLAGS, which is a more general and would basically be another "arguments" to the process, but nobody really explained why not a single environment variable to override the initial guess.

[kobalicek]

Okay, giving up.

[kobalicek]

I have provided an initial code that can be used to fix this issue. I tried to make a code that is as least invasive as possible. It only parses one more environment variable (if present) and checks whether the input value is OK.

If I do PR, does it have a chance to be accepted? Please @jhasse would you be that kind to review the code and write your thoughts?

[fbbdev]

Hello! I've been following the discussion. I support the proposal as I often find myself in the same situation. A wrapper script is a bad workaround in this situation IMO, we are only in need of a very simple way to configure (or override) the guessing logic.

I don't see how an optional, adequately named environment variable controlling a guess could hurt ninja, and many people seem to be asking for something like that.

I would even remove the nUserDefined > n check, if the env var is present it just replaces the guess. This looks sane and useful to me.

Of course I'm just stating an opinion, but I'd like to hear explanations for why not and how would it hurt ninja, not just you can do it another way. Thanks!

[jhasse]

Please @jhasse would you be that kind to review the code and write your thoughts?

As the discussion has got a little bit heated, I think it would be better if someone else would review it and decide on this.

The code looks good, I like the name of the variable and would also drop nUserDefined > n. Also you might think of some way to accept 0 as a valid value (not sure if there's a way with strtol), as there's a PR which adds support for that meaning infinity.

[kobalicek]

@jhasse I initially thought that it would not be safe to pass for example 1000, so I wrote it in a way that you can influence the guess from NINJA_NUM_JOBS variable, but you cannot harm ninja itself by assigning it to some ridiculously high value. It's questionable why not, of course, but I just thought to be safe here and to not introduce anything that would require fixing later.

BTW the discussion got heated, that's true, but I don't see any other maintainer here, so I guess we would have to just figure out ourselves and I personally don't care about wording if we can achieve something here.

[ddevault]

I'm here to voice support as well. Omitting this feature on principle seems a bit like navel gazing to me.

[Salamandar]

This flag is mandatory for me too. Due to AUR and other "automated" build processes when I don't have control of the ninja command line… Or PATH variable.

[ddevault]

Bump. Ran into a need for this again. A wrapper script will not work in this use case for the reasons @Salamandar pointed out. Lacking this is a serious sore point with ninja, and the response from the maintainers is really disappointing. I'm going to start advocating that Linux distros drop ninja outright in favor of samurai, which does offer a feature like this.

Paging maintainers who have commented on this before: @jhasse, @nico

The feature does not contradict any of ninja's stated goals and provides a critical feature which is sorely missed, as evidenced by persistent discussion on the subject for 5 years. The feature is simple to add and easy to maintain and already has patches prepared for it. As far as I can tell, you're just proud to have the phrase "Ninja supports one environment variable to control its behavior" in its docs out of a bizzare distaste for environment variables which has taken hold of a certain misled segment of the unix development community, even for an environment variable whose value proposition is significantly stronger than the only other use of getenv in the codebase.

Even as a particularly opinionated maintainer myself, I think you're betraying your responsibility as one by continuing to fix your eyes affirmedly upon your navels in this respect. Please reconsider.

[ddevault]

Thanks @jonesmz, I did.

[jhasse]

As the discussion has got a little bit heated, I think it would be better if someone else would review it and decide on this.

[ddevault]

@jhasse please nominate another maintainer to field this issue.

[jhasse]

You've already paged nico.

[RJVB]

It's a drop-in replacement which only lacks ninja's feature to couple the number of jobs to system load. So the number of jobs is specified by -j on the commandline, or by `SAMUFLAGS=-jN` in the environment (you can also set -v via that variable).

[Salamandar]

Just make an alias ninja=samu, and you won't event feel the difference.

[Salamandar]

I just created the AUR package ninja-samurai that installs Samu instead of Ninja and adds a symlink as a system-wide alias.

[lilydjwg]

@Salamandar Thank you! I'm beginning to promote ninja-samurai to resolve the haunting resource-exhaustion issue on our build machine and I hope it works well.

For people who wonder, it's simpler to install another package than change a already-installed package by post-install hacks or use a modified package.

Edit: Nope, it doesn't work. We need -l not -j...

[graph]

samurai is a re implementation. Why hasn't someone made a fork? The fork can wrap around the main() function and parse environment variables as needed.

[kobalicek]

I really don't understand why it's such a big deal to accept such feature. A small change that would help many and harm none, and yet ninja maintainers spent more time arguing instead of simply reviewing the code and proposing improvements. This project is lost.

[thiagomacieira]

Why not? Have you actually tried it with QtWebEngine?
I admit I haven't. The bloody project is so damn huge that experimenting with it is really expensive (it'll rebuild significant portions if not everything if you just glanced at it the wrong way).

See qt/qtwebengine@3e9cf09 for how this was worked around in the build system and https://codereview.qt-project.org/c/qt/qtwebengine/+/251319 for how a further update would have passed the incoming total from Make. I needed those changes so I could build QtWebEngine in a 100+ node cluster instead of letting ninja run with an equivalent of -j28 because that's all it could see of my system.

Admittedly, nesting one build tool inside another is never a good idea and there will be rough edges.

Anyway, as the recommended solution is to use a script, can we have this script inside the Ninja repository itself, so it gets installed for most people who need it?

[RJVB]

FWIW, I've been using Samu since about the time of the post Thiago just quoted, and haven't looked back. It's a drop-in replacement and at least at the time it made true the promises ninja itself no longer could.

[ddevault]

Likewise. Ninja is essentially deprecated in favor of Samurai as far as I'm concerned.

[beneschtech]

autoconf/make has stood the test of time. It has many environment variables available to control its behavior. "Write a wrapper script" is an approach I'd probably write one of my guys up for in most situations.

[jinluchang]

Adding another case to support adding this feature:

Currently, it is not possible to specify the number of JOBS when building ninja itself using the officially provided configure.py.

See

ninja/configure.py

Line 202 in 53346f1

parser.add_option('--bootstrap', action='store_true',

It is very common that the end user does not directly control the options passed to ninja. And the default value for NUM_JOBS does not work for some modern machines with too many cores but not that much memory.

[eli-schwartz]

At the end of the day, people have spent 8+ years offering cases to support this feature and the Ninja developers don't want to add it. I'm not sure how adding more cases helps persuade them, in practice.

As far as practical solutions that work today go...

The independent Ninja build language reimplementation https://github.com/michaelforney/samurai supports this via the environment variable SAMUFLAGS. Simply use samu anywhere you'd otherwise use ninja, or install samu as your system /usr/bin/ninja (some linux distros have options to install this competing implementation as the default, or only, ninja).

[RJVB]

Fittingly, the samurai were always the more noble ones ^^