RFR: 8343789: Move mutable nmethod data out of CodeCache [v9]

[Stefan Karlsson]

On Thu, 6 Feb 2025 12:12:02 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> src/hotspot/share/code/nmethod.cpp line 2162:
>> 
>>> 2160:     return nullptr;
>>> 2161:   }
>>> 2162:   return RawAccess<>::oop_load(oop_addr_at(index));
>> 
>> This change is removing the GC barriers and is likely the cause of the ZGC crash that Tobias listed.
>> 
>> However, the fix is not as simple as to just reinstate the NMethodAccess call. The ZGC code uses the `oop*` to find the associated `nmethod` in the code cache. We need another way to fetch the nmethod now. So, I'm experimenting with a small change to switch out the Access API call to a direct GC barrier set call and then I pass down the `this` pointer from this function. With that you should be able skip this change.
>> 
>> With that said, what was the motivation for changing this?
>
> Right, I remember I encountered a "did not find nmethod" assertion in ZNMethod::load_oop. With my change the method fails to find nmethod by oop* which is supposed to be within the boundaries of nmethod. Based on naming semantics, I thought that if the oop wasn't in the nmethod, switching from NMethodAccess to RawAccess was acceptable. In reality, this bypasses the essential GC barrier.
> 
> Given that the nmethod is known in a call stack above, I think we should just pass it down instead of attempting a lookup.
> 
> 
> call stack:
> - ZNMethod::load_oop(oop*, unsigned long)
> - AccessInternal::PostRuntimeDispatch<ZBarrierSet::AccessBarrier<., .>, ., .>::oop_access_barrier(void*)  
> - AccessInternal::RuntimeDispatch<1122372ul, oop, (AccessInternal::BarrierType)2>::load_init(void*)  
> - nmethod::oop_at_phantom(int) 
> 
> oop ZNMethod::load_oop(oop* p, DecoratorSet decorators) {
>   nmethod* const nm = CodeCache::find_nmethod((void*)p);
>   assert(nm != nullptr, "did not find nmethod");

Right, I created a patch to do exactly this and based it upon your PR branch. Take a look at my commit in:
https://github.com/stefank/jdk/tree/rewire_nmethod_oop_loads

I can try to get this upstreamed after I've done enough testing.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r1944723712