Hello Severin,

On 2019-11-08 05:08, Severin Gehwolf wrote:
On Fri, 2019-11-08 at 11:26 +0100, Severin Gehwolf wrote:
On Fri, 2019-11-08 at 09:58 +0100, Magnus Ihse Bursie wrote:
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.
Unfortunately ulimit exists in Cygwin but I it's unclear what it does. On my 32 threads Windows machine, it currently has value 256 for -u, which would severely limit testing for no good reason. I think we need to keep explicitly not doing this on Windows.
Here you go:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8233712/03/webrev/
Hmm, this failed jdk/submit. Could anybody provide me with some
details, please?

[Mach5] mach5-one-sgehwolf-JDK-8233712-3-20191108-1036-6526715: FAILED

The problem is that when we run tests in our distributed system, we don't run configure on the test node, but instead use the "run-test-prebuilt" make target. For any tool configure needs to discover, that is also used in RunTest.gmk, there needs to be a backup discovery method in RunTestPrebuiltSpec.gmk. It's currently just a list of hardcoded values. So add "ULIMIT := ulimit" there and it should work.

/Erik

Thanks,
Severin

Reply via email to