RFR: 8233712: Limit default tests jobs based on ulimit -u setting

[Magnus Ihse Bursie]

On 2019-11-07 20:48, Severin Gehwolf wrote:
> Hi Erik,
>
> On Thu, 2019-11-07 at 10:01 -0800, Erik Joelsson wrote:
>> Hello Severin,
>>
>> Taking ulimit -u into account does seem like a good idea. I don't see
>> why it needs to be limited to just aarch64 though. It should however be
>> limited to OSes that use ulimit (i.e. not windows).
>>
>> I would suggest removing the arbitrary thresholds of 16 and 4096 and try
>> to come up with a plain number based on the ulimit value. You are saying
>> 14 is good for 4096, so the formula for the ULIMIT_DIVIDER is 4096 / 14,
>> which you can just write as:
>>
>> ULIMIT_DIVIER := (4096 / 14)
>>
>> And let the awk script calculate it if needed. In the awk script, you
>> need to put line 275 as conditional of the if statement on line 274.
>> Otherwise ULIMIT=-1 will cause concurrency -1.
> Thanks for the review! Updated webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8233712/02/webrev/
I know I'm coming here and making a simple patch more complicated, but 
I'm not entirely comfortable with the explicit call to ulimit. Our 
principle is that all executables that we call from the makefiles should 
be confirmed existing by configure, so the call should be "$(shell 
$(ULIMIT) -u)".

It should be relatively trivial to add this as a required prog in 
basics.m4 and spec.gmk.in. Make sure you only add it as required for 
non-Windows platforms. If you do this, you can change the check in the 
makefile to if $(ULIMIT) has a value, rather than checking on platform.

/Magnus
>
> What do you think?
>
> Thanks,
> Severin
>
>> /Erik
>>
>> On 2019-11-07 06:59, Severin Gehwolf wrote:
>>> Hi,
>>>
>>> Could I please get a review of this change for running tests on big
>>> Aarch64 boxes? Currently, only memory and number of cores are taken
>>> into account for the -concurrency setting of jtreg. After this patch
>>> ulimit -u settings are taken into account as well on big Aarch64 boxes
>>> with > 16 cores, yet low ulimit -u setting (<= 4096). This is to avoid
>>> running into the max allowed threads limit causing random test
>>> failures.
>>>
>>> Thoughts?
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233712
>>> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8233712/01/webrev/
>>>
>>> Testing: Ran tier1 tests on a large Aarch64 box and low ulimit -u
>>> value. Tests run stable. Did the same for a user with high ulimit -u
>>> value, which resulted in concurrency setting as before this patch.
>>>
>>> Thanks,
>>> Severin
>>>