RFR: 8272107: Removal of Unsafe::defineAnonymousClass left a dangling C++ class

[David Holmes]

On 11/08/2021 8:39 am, David Holmes wrote:
> On 11/08/2021 3:22 am, Ioi Lam wrote:
>> On Tue, 10 Aug 2021 16:37:44 GMT, Coleen Phillimore 
>> <coleenp at openjdk.org> wrote:
>>
>>>> Please review this change to remove the now unused CPSlot class.  
>>>> Uses of CPSlot have been replaced with Symbol*.  The change was 
>>>> tested by running JCK lang and VM tests, Mach5 tiers 1-2 on Linux, 
>>>> Mac OS, and WIndows and Macn5 tiers 3-5 on Linux x64.
>>>>
>>>> Thanks, Harold
>>>
>>> src/hotspot/share/oops/constantPool.inline.hpp line 39:
>>>
>>>> 37:   intptr_t adr = Atomic::load_acquire(obj_at_addr(which));
>>>> 38:   assert(adr != 0 || which == 0, "cp entry for klass should not 
>>>> be zero");
>>>> 39:   return (Symbol*)adr;
>>>
>>> I think you can get rid of all slot_at* and replace them with 
>>> symbol_at().  Since the symbols don't change after the constant pool 
>>> is initialized, all this volatile isn't needed.  Used to be used to 
>>> replace Symbol* with Klass* also.  @iklam can confirm.
>>
>> `ConstantPool` objects are no modified after the class is linked, and 
>> linking is done single threaded. So there's no need to use the 
>> `Atomic::load_acquire` in the above code. I agree with Coleen that the 
>> above function can be replaced with symbol_at().
> 
> The load_acquire should be matching a release_store, so does that mean 
> those can be removed too? I'm not clear whether all linking must pass 
> through a lock, or there are some lock-free fastpaths that might still 
> need acquire/release semantics?

Re-sending to see if email breakage is fixed in PR.

David

> Cheers,
> David
> 
>> -------------
>>
>> PR: https://git.openjdk.java.net/jdk/pull/5070
>>