On Fri, 2019-11-08 at 06:06 -0800, Erik Joelsson wrote: > 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.
Right. I believe webrev 03 does this? > > > 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. Thanks! Tried it, which doesn't seem to work[1]: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8233712/04/webrev/ Is there some way to reproduce locally? Thanks, Severin [1] [Mach5] mach5-one-sgehwolf-JDK-8233712-4-20191108-1454-6533841: FAILED > /Erik > > > Thanks, > > Severin > >
