SA for ZGC

[Yasumasa Suenaga]

Hi Stefan, Per,

Thank you for your comments and patches!
I uploaded new webrev:

   all:
     http://cr.openjdk.java.net/~ysuenaga/z/sa-universe/webrev.01/all/

   diff from Stefan's patch:
     http://cr.openjdk.java.net/~ysuenaga/z/sa-universe/webrev.01/diff/


Stefan's patch seems to have resolved JDK-8194737.
Should we resolve this JDK-8194737 in upstream repo and merge it to zgc at first?


> Just one comment, in ZPage.java:
> 
>    91         // There's no relocate operation in the SA. Simply use the from address and hope for the best.
>    92         return from;
> 
> we should return ZAddress.good(from) here. And then we can remove the last part of the comment. Maybe say something about in-place forwarding instead.

I've fixed.


>> 1) Some of the constants are Linux-specific and there's no platform abstraction in place.

I added some constant values to vmStructs_z.hpp . So we can remove Linux-specific values from SA Java sources.
But I've not remove address() from ZAddress.java because I'm not sure why this method is needed.


>> 2) ZOop.to_address subverts the type system so that we can get from OopHandles and longs to Addresses. This seems to be frowned upon if you read the SA code. Maybe there are more correct ways to implement this?

I changed ZOop#to_address() which have one argument which type is Address.
I guess this change is type-safe.


>> 4) I see that you used VMObjectFactory.newObject, I skipped that part, but we can think about unifying our patches.

I think the classes which extends VMObject should be instantiated via VMObjectFactory#newObject() because other SA codes seems to be so.
I changed to use this factory method as possible.


>> 5) I had to invent a way to read global variables from HotSpot. See the usage of ZGlobalsForVMStructs. Maybe there's a more correct way to do this?

I added these values to constant values in VMStructs.
Thus I removed ZGlobalsForVMStructs.


>> I stress tested the thread dumping part that threw exceptions for me earlier by running SPECjbb2005:
>> $ java -XX:+UseZGC -Xlog:gc -cp jbb.jar:check.jar spec.jbb.JBBmain
>>
>> and then continuously dumping the threads:
>> $ while true; do echo -e "verbose true\nthreads" | jhsdb -J-ea clhsdb --pid=`jps | grep JBBmain | awk '{print $1}'`; done

Sorry, I do not have SPECjbb.
Can you share the result?

I tested `universe` and `jstack` and `jstack -v` on CLHSDB with simple Java app.
This change works fine on my Linux x64 box.


Thanks,

Yasumasa


On 2018/01/11 21:40, Per Liden wrote:
> Hi,
> 
> On 01/10/2018 05:55 PM, Stefan Karlsson wrote:
>> Hi Yasumasa,
>>
>> Here's my first stab at adding the ZGC load barries to the SA:
>>
>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/webrev.01/
> 
> Just one comment, in ZPage.java:
> 
>    91         // There's no relocate operation in the SA. Simply use the from address and hope for the best.
>    92         return from;
> 
> we should return ZAddress.good(from) here. And then we can remove the last part of the comment. Maybe say something about in-place forwarding instead.
> 
> cheers,
> Per
> 
>>
>> I find it unfortunate that we have to duplicate the C++ code into Java, to get this to work. But it is what it is.
>>
>> Some comments about the code:
>>
>> 1) Some of the constants are Linux-specific and there's no platform abstraction in place.
>>
>> 2) ZOop.to_address subverts the type system so that we can get from OopHandles and longs to Addresses. This seems to be frowned upon if you read the SA code. Maybe there are more correct ways to implement this?
>>
>> 3) I use the ZGC weak barrier, since the SA agent only needs to be able to access the Objects, not mark them. Which is what the weak barriers provide.
>>
>> 4) I see that you used VMObjectFactory.newObject, I skipped that part, but we can think about unifying our patches.
>>
>> 5) I had to invent a way to read global variables from HotSpot. See the usage of ZGlobalsForVMStructs. Maybe there's a more correct way to do this?
>>
>> I stress tested the thread dumping part that threw exceptions for me earlier by running SPECjbb2005:
>> $ java -XX:+UseZGC -Xlog:gc -cp jbb.jar:check.jar spec.jbb.JBBmain
>>
>> and then continuously dumping the threads:
>> $ while true; do echo -e "verbose true\nthreads" | jhsdb -J-ea clhsdb --pid=`jps | grep JBBmain | awk '{print $1}'`; done
>>
>>
>> Cheers,
>> StefanK
>>
>> On 2018-01-09 13:59, Yasumasa Suenaga wrote:
>>> Hi Stefan,
>>>
>>>
>>>> Let me try to create an initial prototype for load barriers in the SA case and see where this takes us.
>>>
>>> Sure!
>>>
>>>
>>>> FYI i also found that compressed oops are not fully supported in the SA:
>>>> https://bugs.openjdk.java.net/browse/JDK-8194737
>>>
>>> Thanks, I didn't know that.
>>> I hope this issue will be fixed soon :-)
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2018/01/09 18:36, Stefan Karlsson wrote:
>>>> Hi Yasumasa,
>>>>
>>>> Thanks for your contribution!
>>>>
>>>> I tested your patch together with jhsdb hsdb and immediately hit problems because of the lack of load barriers in the SA code. Let me try to create an initial prototype for load barriers in the SA case and see where this takes us.
>>>>
>>>> FYI i also found that compressed oops are not fully supported in the SA:
>>>> https://bugs.openjdk.java.net/browse/JDK-8194737
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>> On 2017-12-21 23:38, Yasumasa Suenaga wrote:
>>>>> Thanks Per!
>>>>>
>>>>> I'm waiting for comments and sponsorship.
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/12/22 4:33, Per Liden wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> On 2017-12-21 09:40, Yasumasa Suenaga wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I checked SA implementation for ZGC, but it is not yet available.
>>>>>>> For example, we can see WrongTypeException when CLHSDB `universe`
>>>>>>> command is executed.
>>>>>>>
>>>>>>> As the first step, I propose to implement ZCollectedHeap and related classes for
>>>>>>>   SA as this webrev:
>>>>>>>
>>>>>>>    http://cr.openjdk.java.net/~ysuenaga/z/sa-universe/
>>>>>>>
>>>>>>> After applying this patch, we can use `universe` command on CLHSDB.
>>>>>>> (I followed `VM.info` jcmd output format)
>>>>>>
>>>>>> Thanks! Most of us working on ZGC are currently on vacation, but we'll have a closer look at the patch after the holidays.
>>>>>>
>>>>>> cheers,
>>>>>> Per
>>>>>>
>>>>>>>
>>>>>>> Of course, it is not all. We need to work more for it.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>