RFC: 6898462: The escape analysis with G1 cause crash assertion src/share/vm/runtime/vframeArray.cpp:94

[Vladimir Kozlov]

Good :) Really, I almost did not find any serious issues with these changes.

The only question I have: why not pop all interpreter frames 
corresponding to deoptimized compiled frame? You are trying to find the 
top popped frame in vframeArray.cpp. But how we guarantee that frames 
which are not popped are in correct state after deoptimization?

deoptimization.cpp:

I think next assert should be guarantee or fatal:

assert((failed_realloc_from_top != 0) == realloc_failures, "inconsistent 
state");

Add to next comment that inconsistent state of monitors are due to 
failed reallocation of synchronized objects (am I right?):

+     // Unlock all monitors here otherwise the interpreter will see a
+     // mix of locked and unlocked monitors and be confused.

set_removed_monitors() is defined only under ASSERT:

+        array->element(i)->set_removed_monitors();

thread.hpp - fix comment:

+   // If a reallocation of scalar replaced objects fails during 
deoptimization,
+   // we throw OOM and during exception propagation, pop the top
+   // _frames_to_pop_failed_realloc interpreter frames corresponding
+   // to the deoptimized compiler frame.


vframe_hp.cpp

may be:

((ObjectValue *)sv)->value()->is_null()

instead of

((ObjectValue *)sv)->value()() == NULL


TestDeoptOOM.java - @summary is wrong.
Why you exclude OSR? You can exclude main() from compilation instead. 
And we need special OSR subtest to verify that frames are reconstructed 
correctly for OSR compilation. An other case is 2 scalarized allocations 
and one's field references an other.

Thanks,
Vladimir

On 11/18/14 11:29 AM, Roland Westrelin wrote:
> This is the OOM during deoptimization that causes a crash problem.
>
> Here is the change I propose:
>
> http://cr.openjdk.java.net/~roland/6898462/webrev.00/
>
> I haven’t done a lot of testing. I’m looking for feedback.
>
> With that change, the top interpreter frames that reference a failed reallocation are unconditionally popped as the exception is propagated. I record the number of frames to pop in the JavaThread structure and pop the frames in the interpreter runtime code that does the exception propagation.
>
> Roland.
>