Clean up CompiledMethod::oops_reloc_begin()

[Vladimir Kozlov]

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