RFR(s): 8235912: JvmtiBreakpoint remove oops_do and metadata_do

[Robbin Ehn]

Hi Coleen,

On 12/16/19 9:21 PM, coleen.phillimore at oracle.com wrote:
> 
> http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.udiff.html
> 
> + NativeAccess<>::oop_store(_class_holder, class_holder_oop);
> 
> 
> This should probably be:
> 
>    41   NativeAccess<IS_DEST_UNINITIALIZED>::oop_store(handle, obj);
> 

I have not seen any stores to oopStorage that use that?
oopStorage should be 'initialized'.
So I prefer not adding another decorator if it's not needed.
That would just be confusing.

> 
> You can leave out using OopHandle.  I have a patch to add the missing 
> functionality and add it to your code.   Actually, I was looking to see how much 
> OopHandle is used to see if it's helping anything and there is a lot of code 
> using it.  Most of it is to hide oop* in ClassLoaderData.
> 
> This change otherwise looks great.

Thanks, Robbin

> Thanks,
> Coleen
> 
> 
>> Thanks for having a look, Robbin
>>
>> On 12/16/19 1:32 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> I have to think about this.   Could there be breakpoints in old emcp methods 
>>> that we do not remove?   The metadata_do function is trying to keep old 
>>> Methods from being deleted while there are still references to them.
>>>
>>> http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.hpp.udiff.html 
>>>
>>>
>>> + oop* _class_holder; // keeps _method memory from being deallocated
>>>
>>>
>>> We created the class OopHandle to encapsulate strong oopStorage references, 
>>> although it's missing oop_store.  Can you use that?
>>
>>
>>>
>>> Coleen
>>>
>>> On 12/16/19 4:47 AM, Robbin Ehn wrote:
>>>> Hi all, please review.
>>>>
>>>> From issue, https://bugs.openjdk.java.net/browse/JDK-8235912:
>>>>
>>>> JvmtiBreakpoints are walked via VMThread oops_do (the breakpoint is in a vm 
>>>> operation) before they are installed in the safeopint and after they have 
>>>> been installed, walked with JvmtiCurrentBreakpoints::oops_do().
>>>> By putting the class holder inside oopStorage there is no need for this.
>>>>
>>>> JvmtiCurrentBreakpoints::metadata_do is not needed because redefine classes 
>>>> actually removes the breakpoints before updating them (so there is no 
>>>> breakpoints to update).
>>>> We can just remove metadata_do.
>>>>
>>>>
>>>> I also removed some unused code.
>>>>
>>>> Changeset:
>>>> http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/
>>>>
>>>> Passes several runs of nsk jvmti/jdi and t1-7.
>>>>
>>>> Thanks, Robbin
>>>
>