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

[Erik Österlund]

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/

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