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

[Erik Österlund]

Hi Per,

Thanks for the review.

/Erik

On 2018-11-05 10:51, Per Liden wrote:
> 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
>>