RFR(M): 8023014: closed/com/oracle/jfr/compiler/CodeSweeperSweepNoFlushTest.java fails with HS crash

[Vladimir Kozlov]

Thank you, Albert

I would suggest to do TieredStartAtLevel changes separately, it is not 
related to this bug.

Second, how we can guard the initialization with a field 
(_compiler_state and original _is_initialized) which is not static?

You don't need separate local do_init in C*_Compiler.cpp since it is 
used only once:

+  if (should_perform_init()) {

abstractCompiler.cpp:
You removed ThreadInVMfromNative and ResetNoHandleMark from 
should_perform_init() and set_state(). Why? There is assert in 
check_prelock_state() called from Monitor::lock():

   assert((!thread->is_Java_thread() || ((JavaThread 
*)thread)->thread_state() == _thread_in_vm)
          || rank() == Mutex::special, "wrong thread state for using 
locks");

Compiler threads are Java threads, so you should be in VM state when 
lock. How you did not hit this assert?


Typo in abstractCompiler.hpp 'methos':

+  // This method returns true for the first compiler thread that 
reaches that methos.

It is not 'compiler object', it is 'compiler runtime' (yes it was before 
but it was not accurate, it could be mistaking for Compile object 
created for each compilation):

+  // This thread will initialize the compiler object.

Swap lines (and use 'runtime' in the comment):

!   bool is_initialized     ()       { return _compiler_state == 
initialized; }
!   // Get/set state of compiler objects


c2compiler.cpp:

C2Compiler:: is not needed:

+    bool successful = C2Compiler::init_c2_runtime();


Should we exit Compiler thread which failed local buffer allocation 
instead parking? Why keep resources allocated for it?

Thanks,
Vladimir

On 9/25/13 1:05 PM, Albert Noll wrote:
> Hi,
>
> could I have a review for this patch?
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8023014
> webrev: http://cr.openjdk.java.net/~anoll/8023014/webrev.00/
> <http://cr.openjdk.java.net/%7Eanoll/8023014/webrev.00/>
>
> Problem: C1/C2 compiler initialization can fail if there is not enough
> memory in the code cache to generate all stubs.
> This problem occurs only with a small code cache size.
>
> Solution: Check correctness of (parts of) C1/C2 initialization and
> disable compiler (C1 and/or C2) if the compiler / thread
> was not initialized correctly.
>
> Testing: jprt, test case, benchmarks
>
> This patch also contains a refactored version of compiler object/thread
> initialization. In the new version, a compiler object/thread is
> initialized prior entering the main compiler loop. In the old version,
> the check if the compiler/thread is initialized was
> done in the main compiler loop (and hence before every compilation).
>
> To continue execution with a limited code cache size, the code types
> 'AdapterBlob' and 'MethodHandlesAdapterBlob' must
> be 'critical' allocations in the code cache, i.e., they must use space
> reserved by CodeCacheMinimumFreeSpace.
>
> In addition I removed some unued code.
>
> Many thanks in advance for reviewing.
>
> Best,
> Albert