RFR: 8212585: Clean up CompiledMethod::oops_reloc_begin()

[Per Liden]

Hi Erik,

On 10/18/18 11:07 PM, Erik Österlund wrote:
> Hi,
> 
> I decided to make this less risky for platforms that do not yet need to 
> be scanned concurrently, and hence don't really need to be "fixed".
> 
> In this new webrev, I decided to check for the frame completed offset if 
> available and further away than verified entry + native jump size. Then 
> it is safe for me to go ahead and scan this stuff concurrently using 
> nmethod entry barriers. Otherwise, the same good old behaviour we used 
> to rely on applies so that we won't get any surprises where we do not 
> want them.
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8212585/webrev.01/

Looks good!

/Per

> 
> Thanks,
> /Erik
> 
> On 2018-10-17 17:36, 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
>