[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR/C: 8218922: SA: Enable best-effort implementation of live regions iteration for ZGC
From:       Kevin Walls <kevin.walls () oracle ! com>
Date:       2019-02-18 16:07:10
Message-ID: f5330efa-c3c6-569f-2c83-133cb9283224 () oracle ! com
[Download RAW message or body]


Great, thanks Stefan, that collection of changes builds for me and I've 
seem the histogram from a live process using ZGC, and a core of the 
same.   Fantastic. 8-)


On 15/02/2019 19:06, Stefan Karlsson wrote:
> Hi Kevin,
>
> On 2019-02-15 13:14, Kevin Walls wrote:
>> Hi Stefan,
>>
>> This is great - if I want to try it out, what order do the changes 
>> apply, does the 8218746 webrev apply to create ZExternalBitMap.java 
>> and then the 8218922 webrev?
> This is my patch queue of outgoing patches:
>   zSA.fixVMObjectFactory.8218731
>   zSA.fixPhysical.8218732
>   zSA.heapUsedCapacity.8218733
>   zSA.resolveOopHandle.8218734
>   zSA.bitMapsSegmented.8218743
>   zSA.bitMapsZ.8218746
>   zSA.pagesIterate.refactor.8219003
>   zSA.pagesIterate.8218922.Z
>   zSA.turnOnHprof.8218970
>   zSA.turnOnTests.8218978
>
> I've created one big webrev over all these changes:
>   https://cr.openjdk.java.net/~stefank/zgc/zSABitMapsAndLiveRegions/webrev/ 
>
>
> StefanK
>
>>
>> Thanks
>> Kevin
>>
>>
>> On 14/02/2019 17:12, Stefan Karlsson wrote:
>>> Hi again,
>>>
>>> I've separated the live regions iteration refactoring into this patch:
>>> https://cr.openjdk.java.net/~stefank/8219003/webrev.01/
>>>
>>> And use this RFE for the ZGC specific parts:
>>> https://cr.openjdk.java.net/~stefank/8218922/webrev.02/
>>>
>>> Thanks,
>>> StefanK
>>>
>>> On 2019-02-14 14:39, Stefan Karlsson wrote:
>>>> Hi Yasumasa,
>>>>
>>>> On 2019-02-14 14:11, Yasumasa Suenaga wrote:
>>>>> Hi Stefan,
>>>>>
>>>>>> Maybe this is enough to enable a bit more SA debugging 
>>>>>> capabilities when
>>>>>> running with ZGC? What do you think, should we bring in this change?
>>>>>
>>>>> I think it should be brought this in.
>>>>> I filed same issue as JDK-8207843, but I've not yet worked.
>>>>> So I will close it as duplicate of your change.
>>>>>
>>>>>
>>>>>> To be able to implement this more cleanly I've restructured the live
>>>>>> region collection, and pushed GC specific code into the specific 
>>>>>> GCs.
>>>>>> There are some extra usage of generics to make the code a bit 
>>>>>> easier to
>>>>>> read and develop.
>>>>>
>>>>> IMHO refactoring for live region collection and ZGC related 
>>>>> changes should
>>>>> be separated. What do you think?
>>>>
>>>> Yes. I think that would be good. I'll separate this out into two 
>>>> changes.
>>>>
>>>>>
>>>>>
>>>>> Your change looks good to me.
>>>>> BTW, did you check `jhsdb jmap --binaryheap` with this change?
>>>>>
>>>>
>>>> Yes, when testing this I ran all tests in serviceability/sa and 
>>>> manually tested the command above.
>>>>
>>>> While testing this I also had this patch applied to enable SA hprof 
>>>> for ZGC:
>>>> http://cr.openjdk.java.net/~stefank/8218970/webrev.01/
>>>>
>>>> And used this patch to turn off the ZGC filtering in the tests:
>>>> http://cr.openjdk.java.net/~stefank/8218978/webrev.01/
>>>>
>>>> I'm currently rerunning the tests to see that the latest changes 
>>>> didn't break anything.
>>>>
>>>> Thanks,
>>>> StefanK
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> On 2019/02/13 23:52, Stefan Karlsson wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Please review / comment on this patch to enable a best-effort 
>>>>>> live heap
>>>>>> region iteration implementation in ZGC.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~stefank/8218922/webrev.01/
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8218922
>>>>>>
>>>>>> The SA has functionally that relies on live heap region 
>>>>>> information from
>>>>>> the GCs. This is problematic when dead objects are left in the 
>>>>>> heap, and
>>>>>> their classes have been unloaded.
>>>>>>
>>>>>> Because of this ZGC has so far not implemented this feature. 
>>>>>> However, we
>>>>>> could provide a best-effort implementation that most likely will 
>>>>>> work if
>>>>>> classes are not unloaded (or class unloading is turned off), and
>>>>>> otherwise might fail to fully parse and report all live heap 
>>>>>> regions.
>>>>>>
>>>>>> For active, non-relocating pages the patch simply returns [start, 
>>>>>> top)
>>>>>> and for pages being actively relocated, it reports regions 
>>>>>> containing
>>>>>> the non-forwarded objects, the other objects are either dead or 
>>>>>> could be
>>>>>> found in one of the to-regions.
>>>>>>
>>>>>> With this patch I'm able to get heap histograms with ZGC.
>>>>>>
>>>>>> Maybe this is enough to enable a bit more SA debugging 
>>>>>> capabilities when
>>>>>> running with ZGC? What do you think, should we bring in this change?
>>>>>>
>>>>>> To be able to implement this more cleanly I've restructured the live
>>>>>> region collection, and pushed GC specific code into the specific 
>>>>>> GCs.
>>>>>> There are some extra usage of generics to make the code a bit 
>>>>>> easier to
>>>>>> read and develop.
>>>>>>
>>>>>> Thanks,
>>>>>> StefanK
>>>>>>
>>>
>
[prev in list] [next in list] [prev in thread] [next in thread]