Clean up CompiledMethod::oops_reloc_begin()

[Erik Osterlund]

Hi Vladimir,

Thanks for the review.

/Erik

> On 19 Oct 2018, at 23:38, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Hi Erik,
> 
> I think it was simple optimization to skip jump instruction - nothing more.
> 
> Yes, there is special handling of Verified entry code generation to make sure there is space for jump instructions there - there is no way oop will be there:
> 
> http://hg.openjdk.java.net/jdk/jdk/file/4d1e5697b32b/src/hotspot/cpu/x86/nativeInst_x86.cpp#l540
> http://hg.openjdk.java.net/jdk/jdk/file/4d1e5697b32b/src/hotspot/cpu/x86/macroAssembler_x86.cpp#l5455
> 
> Yes, code in zNMethodTable.cpp is not needed. And change in compiledMethod.cpp is good. Reviewed.
> 
> Thanks,
> Vladimir
> 
>> On 10/17/18 8:36 AM, Erik Österlund wrote:
>> Hi,
>> In CompiledMethod::oops_reloc_begin() there is code to prevent looking for oops at the verified entry of an nmethod that is not entrant, as a native jump instruction has been overwritten there. Except there would *never* be any immediate oops there for any compiler, even before becoming not entrant, with or without OSR compilation, so this special handling of not entrant vs entrant nmethods is seemingly completely unnecessary.
>> This gets increasingly awkward when oops_do is called concurrently with concurrent class unloading, where an nmethod is racing to become not entrant.
>> To clean this up, I propose to change the boundary for immediate oop scanning and start looking for oops only after the frame building is completed, as there is absolutely no point in looking for oops before that in the first place. This removes unnecessary workarounds that exist today, and removes unnecessary races going forward.
>> It seems like Graal as JIT also inlines oops into the code stream, but never sets the CodeOffsets::Frame_Complete in the nmethod. So I recognize such nmethods and unconditionally start scanning at the verified entry + native jump instruction size. I spoke to the Graal folks, and they said they do not put oops in there either. So I think everyone involved should be happy with this solution.
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8212585
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8212585/webrev.00/
>> Thanks,
>> /Erik