Number of make jobs for bootcycle-images target

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Number of make jobs for bootcycle-images target

Alex Kashchenko
Hi,

Currently in jdk9 bootcycle-images make target is executed with
unlimited number of make jobs. May I ask whether "JOBS=" bit here [1] is
intentional or just a typo and should be "JOBS=$(JOBS)" instead?

Current variant works on x86_64 but crashes with native arm32 boot cycle
builds - compilation tasks are spawned faster than being executed.


[1]
http://hg.openjdk.java.net/jdk9/jdk9/file/41d9f0545d53/make/Main.gmk#l323

--
-Alex
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Number of make jobs for bootcycle-images target

Erik Joelsson
Hello Alex,

It wasn't a typo, but it was also not correct, as you are pointing out.
Setting JOBS to $(JOBS) would disable the jobserver for the sub make
process and also risk flooding a smaller system. What we really need is
a way to block the setting of -j in the "Init.gmk main" target.
Something like this seems to work for me:


diff -r 7810f75d016a make/Init.gmk
--- a/make/Init.gmk
+++ b/make/Init.gmk
@@ -303,7 +303,8 @@
          $(call PrepareSmartJavac)
          ( cd $(TOPDIR) && \
              $(NICE) $(MAKE) $(MAKE_ARGS) $(OUTPUT_SYNC_FLAG) \
-                -j $(JOBS) -f make/Main.gmk $(USER_MAKE_VARS) \
+                $(if $(DISABLE_JOBS),, -j $(JOBS)) \
+                -f make/Main.gmk $(USER_MAKE_VARS) \
                  $(PARALLEL_TARGETS) $(COMPARE_BUILD_MAKE)
$(BUILD_LOG_PIPE) || \
              ( exitcode=$$? && \
              $(PRINTF) "\nERROR: Build failed for $(TARGET_DESCRIPTION)
(exit code $$exitcode) \n" \
diff -r 7810f75d016a make/Main.gmk
--- a/make/Main.gmk
+++ b/make/Main.gmk
@@ -320,7 +320,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 $(SRC_ROOT)/make/Init.gmk
PARALLEL_TARGETS=$(BOOTCYCLE_TARGET) \
-          JOBS= SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
+          DISABLE_JOBS=true SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
          else
        $(call LogWarn, Boot cycle build disabled when cross compiling)
          endif


/Erik

On 2017-04-04 13:26, Alex Kashchenko wrote:

> Hi,
>
> Currently in jdk9 bootcycle-images make target is executed with
> unlimited number of make jobs. May I ask whether "JOBS=" bit here [1]
> is intentional or just a typo and should be "JOBS=$(JOBS)" instead?
>
> Current variant works on x86_64 but crashes with native arm32 boot
> cycle builds - compilation tasks are spawned faster than being executed.
>
>
> [1]
> http://hg.openjdk.java.net/jdk9/jdk9/file/41d9f0545d53/make/Main.gmk#l323
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Number of make jobs for bootcycle-images target

Magnus Ihse Bursie
What is the intention here? You want build the second part of the boot
cycle build without setting -j? Is this because we already have a good
value of -j inherited from an earlier make call?

The fix looks scary, but that's maybe because all of this make-wrapping
logic is scary.

What if you keep sending JOBS= and then check if JOBS has a value in
Init.gmk, instead of introducing DISABLE_JOBS?

/Magnus

On 2017-04-04 14:19, Erik Joelsson wrote:

> Hello Alex,
>
> It wasn't a typo, but it was also not correct, as you are pointing
> out. Setting JOBS to $(JOBS) would disable the jobserver for the sub
> make process and also risk flooding a smaller system. What we really
> need is a way to block the setting of -j in the "Init.gmk main"
> target. Something like this seems to work for me:
>
>
> diff -r 7810f75d016a make/Init.gmk
> --- a/make/Init.gmk
> +++ b/make/Init.gmk
> @@ -303,7 +303,8 @@
>          $(call PrepareSmartJavac)
>          ( cd $(TOPDIR) && \
>              $(NICE) $(MAKE) $(MAKE_ARGS) $(OUTPUT_SYNC_FLAG) \
> -                -j $(JOBS) -f make/Main.gmk $(USER_MAKE_VARS) \
> +                $(if $(DISABLE_JOBS),, -j $(JOBS)) \
> +                -f make/Main.gmk $(USER_MAKE_VARS) \
>                  $(PARALLEL_TARGETS) $(COMPARE_BUILD_MAKE)
> $(BUILD_LOG_PIPE) || \
>              ( exitcode=$$? && \
>              $(PRINTF) "\nERROR: Build failed for
> $(TARGET_DESCRIPTION) (exit code $$exitcode) \n" \
> diff -r 7810f75d016a make/Main.gmk
> --- a/make/Main.gmk
> +++ b/make/Main.gmk
> @@ -320,7 +320,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 $(SRC_ROOT)/make/Init.gmk
> PARALLEL_TARGETS=$(BOOTCYCLE_TARGET) \
> -          JOBS= SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
> +          DISABLE_JOBS=true SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
>          else
>        $(call LogWarn, Boot cycle build disabled when cross compiling)
>          endif
>
>
> /Erik
>
> On 2017-04-04 13:26, Alex Kashchenko wrote:
>> Hi,
>>
>> Currently in jdk9 bootcycle-images make target is executed with
>> unlimited number of make jobs. May I ask whether "JOBS=" bit here [1]
>> is intentional or just a typo and should be "JOBS=$(JOBS)" instead?
>>
>> Current variant works on x86_64 but crashes with native arm32 boot
>> cycle builds - compilation tasks are spawned faster than being executed.
>>
>>
>> [1]
>> http://hg.openjdk.java.net/jdk9/jdk9/file/41d9f0545d53/make/Main.gmk#l323
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Number of make jobs for bootcycle-images target

Erik Joelsson
Hello,


On 2017-04-04 14:59, Magnus Ihse Bursie wrote:
> What is the intention here? You want build the second part of the boot
> cycle build without setting -j? Is this because we already have a good
> value of -j inherited from an earlier make call?
>
We only want to set the -j flag once since resetting it in a sub make
call causes us to loose the job server. If there was some other way of
detecting that we are in a wrapped call, that would certainly help.
> The fix looks scary, but that's maybe because all of this
> make-wrapping logic is scary.
>
> What if you keep sending JOBS= and then check if JOBS has a value in
> Init.gmk, instead of introducing DISABLE_JOBS?
>
I considered that, but then we lose the currently valid empty value for
JOBS, which today translates to -j without argument. One could argue
that it's rarely a good idea to let make just go crazy with processes
though and in that case we can choose to interpret JOBS="" as don't set
-j at all.

/Erik

> /Magnus
>
> On 2017-04-04 14:19, Erik Joelsson wrote:
>> Hello Alex,
>>
>> It wasn't a typo, but it was also not correct, as you are pointing
>> out. Setting JOBS to $(JOBS) would disable the jobserver for the sub
>> make process and also risk flooding a smaller system. What we really
>> need is a way to block the setting of -j in the "Init.gmk main"
>> target. Something like this seems to work for me:
>>
>>
>> diff -r 7810f75d016a make/Init.gmk
>> --- a/make/Init.gmk
>> +++ b/make/Init.gmk
>> @@ -303,7 +303,8 @@
>>          $(call PrepareSmartJavac)
>>          ( cd $(TOPDIR) && \
>>              $(NICE) $(MAKE) $(MAKE_ARGS) $(OUTPUT_SYNC_FLAG) \
>> -                -j $(JOBS) -f make/Main.gmk $(USER_MAKE_VARS) \
>> +                $(if $(DISABLE_JOBS),, -j $(JOBS)) \
>> +                -f make/Main.gmk $(USER_MAKE_VARS) \
>>                  $(PARALLEL_TARGETS) $(COMPARE_BUILD_MAKE)
>> $(BUILD_LOG_PIPE) || \
>>              ( exitcode=$$? && \
>>              $(PRINTF) "\nERROR: Build failed for
>> $(TARGET_DESCRIPTION) (exit code $$exitcode) \n" \
>> diff -r 7810f75d016a make/Main.gmk
>> --- a/make/Main.gmk
>> +++ b/make/Main.gmk
>> @@ -320,7 +320,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 $(SRC_ROOT)/make/Init.gmk
>> PARALLEL_TARGETS=$(BOOTCYCLE_TARGET) \
>> -          JOBS= SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
>> +          DISABLE_JOBS=true SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
>>          else
>>        $(call LogWarn, Boot cycle build disabled when cross compiling)
>>          endif
>>
>>
>> /Erik
>>
>> On 2017-04-04 13:26, Alex Kashchenko wrote:
>>> Hi,
>>>
>>> Currently in jdk9 bootcycle-images make target is executed with
>>> unlimited number of make jobs. May I ask whether "JOBS=" bit here
>>> [1] is intentional or just a typo and should be "JOBS=$(JOBS)" instead?
>>>
>>> Current variant works on x86_64 but crashes with native arm32 boot
>>> cycle builds - compilation tasks are spawned faster than being
>>> executed.
>>>
>>>
>>> [1]
>>> http://hg.openjdk.java.net/jdk9/jdk9/file/41d9f0545d53/make/Main.gmk#l323
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Number of make jobs for bootcycle-images target

Magnus Ihse Bursie


On 2017-04-04 15:10, Erik Joelsson wrote:

> Hello,
>
>
> On 2017-04-04 14:59, Magnus Ihse Bursie wrote:
>> What is the intention here? You want build the second part of the
>> boot cycle build without setting -j? Is this because we already have
>> a good value of -j inherited from an earlier make call?
>>
> We only want to set the -j flag once since resetting it in a sub make
> call causes us to loose the job server. If there was some other way of
> detecting that we are in a wrapped call, that would certainly help.
There are ways of detecting if we're in a recursive call, but they too
tend to be scary to check. :)

>> The fix looks scary, but that's maybe because all of this
>> make-wrapping logic is scary.
>>
>> What if you keep sending JOBS= and then check if JOBS has a value in
>> Init.gmk, instead of introducing DISABLE_JOBS?
>>
> I considered that, but then we lose the currently valid empty value
> for JOBS, which today translates to -j without argument. One could
> argue that it's rarely a good idea to let make just go crazy with
> processes though and in that case we can choose to interpret JOBS=""
> as don't set -j at all.
I don't think we've really had that as a supported option, have we? We
set JOBS in the spec.gmk based on configure's idea of the ideal number
of jobs for the machine. This idea here is to more or less take control
from make, which we don't trust to make that decision. So unless the
user explicitly sets JOBS= on the command line, this is not a situation
that would occur otherwise. I thought we even checked that JOBS had a
valid number in it, but I couldn't find such a check now so maybe I just
dreamed that one.

/Magnus

>
> /Erik
>
>> /Magnus
>>
>> On 2017-04-04 14:19, Erik Joelsson wrote:
>>> Hello Alex,
>>>
>>> It wasn't a typo, but it was also not correct, as you are pointing
>>> out. Setting JOBS to $(JOBS) would disable the jobserver for the sub
>>> make process and also risk flooding a smaller system. What we really
>>> need is a way to block the setting of -j in the "Init.gmk main"
>>> target. Something like this seems to work for me:
>>>
>>>
>>> diff -r 7810f75d016a make/Init.gmk
>>> --- a/make/Init.gmk
>>> +++ b/make/Init.gmk
>>> @@ -303,7 +303,8 @@
>>>          $(call PrepareSmartJavac)
>>>          ( cd $(TOPDIR) && \
>>>              $(NICE) $(MAKE) $(MAKE_ARGS) $(OUTPUT_SYNC_FLAG) \
>>> -                -j $(JOBS) -f make/Main.gmk $(USER_MAKE_VARS) \
>>> +                $(if $(DISABLE_JOBS),, -j $(JOBS)) \
>>> +                -f make/Main.gmk $(USER_MAKE_VARS) \
>>>                  $(PARALLEL_TARGETS) $(COMPARE_BUILD_MAKE)
>>> $(BUILD_LOG_PIPE) || \
>>>              ( exitcode=$$? && \
>>>              $(PRINTF) "\nERROR: Build failed for
>>> $(TARGET_DESCRIPTION) (exit code $$exitcode) \n" \
>>> diff -r 7810f75d016a make/Main.gmk
>>> --- a/make/Main.gmk
>>> +++ b/make/Main.gmk
>>> @@ -320,7 +320,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 $(SRC_ROOT)/make/Init.gmk
>>> PARALLEL_TARGETS=$(BOOTCYCLE_TARGET) \
>>> -          JOBS= SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
>>> +          DISABLE_JOBS=true SPEC=$(dir $(SPEC))bootcycle-spec.gmk main
>>>          else
>>>        $(call LogWarn, Boot cycle build disabled when cross compiling)
>>>          endif
>>>
>>>
>>> /Erik
>>>
>>> On 2017-04-04 13:26, Alex Kashchenko wrote:
>>>> Hi,
>>>>
>>>> Currently in jdk9 bootcycle-images make target is executed with
>>>> unlimited number of make jobs. May I ask whether "JOBS=" bit here
>>>> [1] is intentional or just a typo and should be "JOBS=$(JOBS)"
>>>> instead?
>>>>
>>>> Current variant works on x86_64 but crashes with native arm32 boot
>>>> cycle builds - compilation tasks are spawned faster than being
>>>> executed.
>>>>
>>>>
>>>> [1]
>>>> http://hg.openjdk.java.net/jdk9/jdk9/file/41d9f0545d53/make/Main.gmk#l323
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Number of make jobs for bootcycle-images target

Alex Kashchenko
In reply to this post by Erik Joelsson
On 04/04/2017 01:19 PM, Erik Joelsson wrote:
> Hello Alex,
>
> It wasn't a typo, but it was also not correct, as you are pointing out.
> Setting JOBS to $(JOBS) would disable the jobserver for the sub make
> process and also risk flooding a smaller system. What we really need is
> a way to block the setting of -j in the "Init.gmk main" target.
> Something like this seems to work for me:
>
> [...]

Thanks! This patch worked for me with bootcycle builds on arm32 and
other archs.

--
-Alex
Loading...