RFR: 8201788: Number of make jobs wrong for bootcycle-images target (original) (raw)

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Thu Apr 19 18:58:52 UTC 2018

Looks good to me too. Thanks for doing it this way.

/Magnus

19 apr. 2018 kl. 20:08 skrev Severin Gehwolf <sgehwolf at redhat.com>:

Hi Erik,

On Thu, 2018-04-19 at 09:03 -0700, Erik Joelsson wrote: Hello,

On 2018-04-19 08:58, Severin Gehwolf wrote: Hi Erik,

Thanks for the review!

On Thu, 2018-04-19 at 08:25 -0700, Erik Joelsson wrote: Hello Severin,

The suggested patch is not a good idea because by setting -j on the make command line in a sub make disables the job server. The job server is what makes it possible to do recursive make with a fixed global number of "jobs". If you do as you suggest, you essentially double the total number of available "jobs". The original make retains its number and the submake get a full other set of the same number of "jobs". I'm confused. Isn't this what the status quo is? With the difference that it's currently setting JOBS="", thus allowing sub-make to add any number of jobs. It'll result in sub-make calling "make -j" where '-j' won't get an argument. If that's the case it's disabling the job server currently too, no? Then again, why would we see build failures? I must be missing something. Ah, correct, the current code is also disabling the job server, that is the core of the issue. :) I'm sorry I wasn't clear on that, it was just so obvious in my world. Any -j flag in a sub make disables the job server connection between the calling make an the sub make. Setting it to -j without argument is going to wreck a lot more havoc than setting it to something like close to "number-of-cpus", which your first suggestion does. The former more or less creates a fork bomb, while the latter only over allocates by a factor 2 at the worst. OK. That does make it sound like that "disabling the job server" and creating more jobs are independent problems. I somehow thought in my naive world that disabling the job server puts an end to the fork-bomb ;-) Thanks for the clarification. My suggestion was to explicitly turn off the setting of JOBS based on a special variable flag, just for bootcycle builds. Magnus didn't like that because introducing a lot of special flags everywhere will eventually lead to very convoluted code. He instead suggested that the bootcycle call should continue to set JOBS to empty, then the code in Init.gmk which sets the -j flag should be changed to: $(if (JOBS),−j=(JOBS), -j=(JOBS),−j=(JOBS)) So that we only set -j if JOBS have a value. My only objection to that was that we then no longer support the case of letting make run with any number of jobs. I do agree that removing that option isn't a big deal. You can always work around it by setting JOBS to a very large number (like 1000, which is way more than any possible concurrency currently possible in the build anyway). So to summarize, I think the correct solution to the bug is the snippet above. Alright. How does this webrev look to you? http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8201788/webrev.01/ Yes, this looks good. Consider it reviewed. Great, thanks for the review! I'm currently running this through jdk- submit. Hopefully I'll get some response this time :) Cheers, Severin /Erik Thanks, Severin /Erik

On 2018-04-19 07:46, Severin Gehwolf wrote: Hi, Bug: https://bugs.openjdk.java.net/browse/JDK-8201788 I'd like to get a fix in for an old discussion which already happened a while ago: http://mail.openjdk.java.net/pipermail/build-dev/2017-April/018929.html The issue is that for bootcycle-images target a recursive call to make is being called with 'JOBS=""' which results in a call to "make -j". Thus, make is free to use as many jobs as it would like. This may cause for the occasional build failure. It has for us in the past. In this old thread above a patch like this was discouraged, unless I misunderstood something. diff --git a/make/Main.gmk b/make/Main.gmk --- a/make/Main.gmk +++ b/make/Main.gmk @@ -321,7 +321,7 @@ ifneq ($(COMPILETYPE), cross) $(call LogWarn, Boot cycle build step 2: Building a new JDK image using previously built image) _+$(MAKE) (MAKEARGS)−f(MAKEARGS) -f (MAKEARGS)−f(TOPDIR)/make/Init.gmk PARALLELTARGETS=$(BOOTCYCLETARGET) _ - JOBS= SPEC=$(dir $(SPEC))bootcycle-spec.gmk main + JOBS=$(JOBS) SPEC=$(dir $(SPEC))bootcycle-spec.gmk main else $(call LogWarn, Boot cycle build disabled when cross compiling) endif It's my understanding that such a fix would pass down the relevant JOBS setting to sub-make and, thus, producing the desired 'make -j ' call? What am I missing? If somebody wants to shoot themselves in the foot by doing: configure ... make JOBS= That should be fine as it would just result in "make -j" calls without arguments. The common case where the JOBS setting comes from configure would be fixed, though. bootcycle-images target would result in "make -j ". Thoughts? Suggestions? Thanks, Severin

More information about the build-dev mailing list