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
