RFR: 8201788: Number of make jobs wrong for bootcycle-images target

[Severin Gehwolf]

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.

> 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))
> 
> 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/

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 ($(COMPILE_TYPE), cross)
> >            $(call LogWarn, Boot cycle build step 2: Building a new JDK image using previously built image)
> >            +$(MAKE) $(MAKE_ARGS) -f $(TOPDIR)/make/Init.gmk PARALLEL_TARGETS=$(BOOTCYCLE_TARGET) \
> > -             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 <JOBS>'
> > 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 <num-cores>".
> > 
> > Thoughts? Suggestions?
> > 
> > Thanks,
> > Severin
> 
>