RFR: 8212585: Clean up CompiledMethod::oops_reloc_begin()
martin.doerr at sap.com
Mon Nov 12 15:29:53 UTC 2018
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,
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()
Thanks for the review.
On 2018-11-05 10:51, Per Liden wrote:
> Hi Erik,
> On 10/18/18 11:07 PM, Erik Österlund wrote:
>> 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.
> Looks good!
>> On 2018-10-17 17:36, Erik Österlund wrote:
>>> 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.
More information about the hotspot-dev