RFR: 8212585: Clean up CompiledMethod::oops_reloc_begin()
Doerr, Martin
martin.doerr at sap.com
Mon Nov 12 15:29:53 UTC 2018
Hi Erik,
I still see crashes on SPARC and PPC64 when running jck tests:
V [libjvm.so+0xef36a0] void DependencyContext::remove_dependent_nmethod(nmethod*,bool)+0x90
V [libjvm.so+0x1243688] void InstanceKlass::remove_dependent_nmethod(nmethod*,bool)+0xa8
V [libjvm.so+0x1b19978] void nmethod::flush_dependencies(bool)+0x388
V [libjvm.so+0x1b1842c] void nmethod::make_unloaded()+0x8c
V [libjvm.so+0x1c249cc] void CodeCacheUnloadingTask::work(unsigned)+0x7c
V [libjvm.so+0x1c24cf0] void ParallelCleaningTask::work(unsigned)+0x10
V [libjvm.so+0x21330b4] void GangWorker::loop()+0xa4
V [libjvm.so+0x1f94908] void Thread::call_run()+0x128
V [libjvm.so+0x1bc297c] thread_native_entry+0x3ec
I haven't tried to debug, but I found a problem by reading code:
We shouldn't use NativeJump::instruction_size in CompiledMethod::oops_reloc_begin().
On SPARC and PPC64, this size is the size of a large instruction sequence, but NativeJump::patch_verified_entry only patches a 4 byte trapping instruction. AFAICS only 4 byte is guaranteed to contain to oop (on these 2 platforms, other ones look ok).
We could fix this constant on PPC64, but SPARC uses is for other purposes.
Do you have a good proposal to fix it?
Thanks and best regards,
Martin
-----Original Message-----
From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of Erik Österlund
Sent: Montag, 5. November 2018 11:14
To: Per Liden <per.liden at oracle.com>; hotspot-dev developers <hotspot-dev at openjdk.java.net>
Subject: Re: RFR: 8212585: Clean up CompiledMethod::oops_reloc_begin()
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
>>
More information about the hotspot-dev
mailing list