For what it's worth Daniil, Webrev.02 LGTM :-)
I do believe at some point we go seem to go back and forth about JVMCI testing and what we should do or not. I understand it should be a case by case in a lot of spots but perhaps we should document somewhere our stance for JVMCI test-bugs? Just food for thought? Thanks, Jc On Fri, Mar 22, 2019 at 8:26 PM Daniil Titov <[email protected]> wrote: > Thank you, Chris! > > Please review a new version of the change that makes the test ignored if > Graal is enabled. > > Webrev: http://cr.openjdk.java.net/~dtitov/8217827/webrev.02/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8217827 > > Best regards, > Daniil > > > > On 3/22/19, 7:59 PM, "Chris Plummer" <[email protected]> wrote: > > Hi Daniil, > > So 8mb is enough to do at least 10,000 iterations and trigger JVMCI > initialization, but the amount of memory needed after the System.gc() > is > more than the memory used by the loop (and then freed)? I wonder if > more > compilation is being triggered after the System.gc() call, and that > uses > a lot of memory. > > Also, I'm not comfortable with this concept of considering JVMCI to be > initialized. You're making assumptions on the internal state of JVMCI. > Other compilations could require other allocations that could end up > failing. You also don't know how JVMCI behavior might change in the > future, causing this test to fail again. Perhaps it is best not to run > these ResourceExhausted tests with JVMCI. Their reliability is dubious > enough already. > > thanks, > > Chris > > On 3/22/19 4:02 PM, Daniil Titov wrote: > > Hi Chris, > > > > Addind -XX:+PrintCompilation flag shows that the first compiled > method is java.lang.Object::<init>. > > > > Max heap size and parameter for the warmup stage (10K iterations) > were found a posteriori, to ensure that JVMCI initialization is kicked but > without throwing OutOfMemoryError. > > > > The heap increase is required otherwise a second OOME is thrown in > the main thread after line 75 and in some cases even after line 86. It > seems as JVMCI eats out all 8Mb of the heap. > > > > 75 System.gc(); > > 76 if ( ! Helper.checkResult("creating " + count + " > objects") ) > > 77 return Consts.TEST_FAILED; > > 78 > > 79 return Consts.TEST_PASSED; > > 80 } > > 81 > > 82 public static void main(String[] args) { > > 83 args = nsk.share.jvmti.JVMTITest.commonInit(args); > > 84 > > 85 int result = run(args, System.out); > > 86 System.out.println(result == Consts.TEST_PASSED ? > "TEST PASSED" : "TEST FAILED"); > > 87 System.exit(result + Consts.JCK_STATUS_BASE); > > 88 } > > > > Best regards, > > Daniil > > > > On 3/22/19, 2:09 PM, "Chris Plummer" <[email protected]> > wrote: > > > > Hi Daniil, > > > > What triggers the JVMCI initialization, what (in general) is > done during > > the initialization, and how did you come up with the 10k > iterations and > > a 10s sleep to ensure that initialization is complete? > > > > What was the reason for the heap size increase? Does the OOME > happen > > before 10k iterations if you don't increase the heap size? > > > > thanks, > > > > Chris > > > > On 3/22/19 1:53 PM, Daniil Titov wrote: > > > Please review the change that fixes the failure of the test > when running with Graal. > > > > > > The problem here is that the test consumes all memory before > JVMCI runtime is fully initialized. As a result the call to > JVMCIRuntime::get_HotSpotJVMCIRuntime(CHECK_EXIT) > > > at src/hotspot/share/jvmci/jvmciCompiler.cpp:132 throws > OutOfmemoryError that is caught by CHECK_EXIT macro that in turn calls > JVMCICompiler::exit_on_pending_exception that performs vm_exit(-1). > > > > > > The fix increases the maximum heap size the test uses and > adds a delay to ensure the JVMCI Runtime is fully initialized before > proceeding with provoking OutOfMemoryError. > > > > > > Before the change the test failure rate in Mach5 builds was > about 25% . With this change after 900 rounds in Mach5 no failure was > detected. The test execution time with the change is 50 second. > > > > > > Webrev: http://cr.openjdk.java.net/~dtitov/8217827/webrev.01/ > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8217827 > > > > > > Thanks! > > > --Daniil > > > > > > > > > > > > > > > > > > > > -- Thanks, Jc
