Request for reviews (M): 6833129 : specjvm98 fails with NullPointerException in the compiler with -XX:DeoptimizeALot
Thomas.Rodriguez at Sun.COM
Tue Jul 7 13:33:32 PDT 2009
I think the term reexecute should be used in place of restart since
that terminology is used elsewhere. Actually I think should_reexecute
is better than is_reexecute as well.
I don't really like that the restart bit is associated with the bci.
It implies that any scope can be reexecuted when it fact it's only the
topmost one that can be. Given how these describing scopes is
structured I'm not sure I see a better way though. I don't see any
asserts to enforce this for the scopeDescs either.
The printing forms for ScopeDesc and JVMState should indicate if this
is a restarted bytecode or not. The SA also needs to be updated to
read these modified ScopeDescs.
>> We talked about using PreserveJVMState pjvms(this) instead
>> of restoring stack and restart flag in intrinsics?
>> Why you did not use it?
> Just as we discussed a moment ago, I added an assert restart false
> we set it to true.
I think manually toggling the restart bit back and forth should be
avoided. Preserve the original and pass on the modified one or have a
helper object that toggles the bit in it's constructor/destructor.
> Here is the updated webrev link!
>> changpeng fang - Sun Microsystems - Santa Clara United States wrote:
>>> The problem is in intrinsics Object.clone and Arrays.copyOf. When
>>> de-optimization occurs on the slow path for array/instance
>>> allocation, the Interpreter will continue execution in next bc
>>> after the slow allocation calls without actual copying.
>>> Add a restart bit in debuginfo to direct the deopt to restart
>>> execution of the bytecode that invokes Object.clone/Arrays.copyOf.
>>> The restart bit is set up in Inline Intrinsics of Object.clone/
>>> Passed specjvm98, JPRT and the test case of clone in the bug report.
More information about the hotspot-compiler-dev