RFR (S) 8251302: Create dedicated OopStorages for Management and Jvmti

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Mon Aug 10 23:29:35 UTC 2020


Coleen,

The update looks good to me.

Thanks,
Serguei


On 8/10/20 15:39, Coleen Phillimore wrote:
> Adding back serviceability-dev.
> Thanks for reviewing Serguei.
> Coleen
>
> On 8/10/20 6:11 PM, Coleen Phillimore wrote:
>>
>>
>> On 8/10/20 5:28 PM, Coleen Phillimore wrote:
>>>
>>>
>>> On 8/10/20 4:38 PM, serguei.spitsyn at oracle.com wrote:
>>>> On 8/10/20 13:34, serguei.spitsyn at oracle.com wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> It looks good to me.
>>>>> Minor:
>>>>> +void JvmtiExport::initialize_oop_storage() {
>>>>> + // OopStorage needs to be created early in startup and 
>>>>> unconditionally
>>>>> + // because of OopStorageSet static array indices.
>>>>> + _jvmti_oop_storage = OopStorageSet::create_strong("Thread 
>>>>> OopStorage");
>>>>> +}
>>>>> +
>>>>> Would it better to replace "Thread Oopstorage" with "JVMTI 
>>>>> OopStorage"?
>>>>
>>>> In the file
>>>> http://cr.openjdk.java.net/~coleenp/2020/8251302.01/webrev/test/jdk/jdk/jfr/event/gc/collection/TestG1ParallelPhases.java.udiff.html 
>>>>
>>>> I see this:
>>>>               "Thread OopStorage",
>>>> + "ThreadService OopStorage",
>>>> It is not clear if we can simply add ""JVMTI OopStorage" above.
>>>
>>> Serguei,  Thank you for finding this.  I was wondering why I didn't 
>>> have to add JVMTI OopStorage to the test.  I'd cut/pasted the same 
>>> string for Thread OopStorage.
>>>
>>> I'll fix this and the test and retest.
>>
>> Hi Serguei,
>>
>> open webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8251302.02.incr/webrev
>>
>> This fixes the test as well.
>>
>> Thanks!
>> Coleen
>>
>>>
>>> thanks,
>>> Coleen
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>> No need in another webrev.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>> On 8/10/20 12:37, Coleen Phillimore wrote:
>>>>>> These OopHandles are created and released during breakpoints and 
>>>>>> Thread stack walking operations.  They should have their own 
>>>>>> OopStorage so that GC can detect whether these things affect timing.
>>>>>>
>>>>>> Tested with tier1-6.
>>>>>>
>>>>>> open webrev at 
>>>>>> http://cr.openjdk.java.net/~coleenp/2020/8251302.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8251302
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>
>>>>
>>>
>>
>



More information about the serviceability-dev mailing list