SA for ZGC
Stefan Karlsson
stefan.karlsson at oracle.com
Thu Jan 18 12:28:30 UTC 2018
FYI: I just pushed this.
http://hg.openjdk.java.net/zgc/zgc/rev/8609ea491452
ZGC: Basic SA support
Contributed-by: stefan.karlsson at oracle.com, yasuenag at gmail.com
Thanks,
StefanK
On 2018-01-17 15:48, Stefan Karlsson wrote:
> Hi Yasumasa,
>
> On 2018-01-17 15:00, Yasumasa Suenaga wrote:
>> Hi Stefan,
>>
>>> I've made some unifications, cleanups, and fixes to the patch:
>>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/webrev.04.delta
>>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/webrev.04
>>
>> Thanks!
>> Just one nit:
>>
>> ZGlobals.java
>>
>> ```
>> 85 ZAddressOffsetBits =
>> db.lookupLongConstant("ZAddressOffsetBits").longValue();
>> 86 ZAddressOffsetMask =
>> db.lookupLongConstant("ZAddressOffsetMask").longValue();
>> 87 System.out.println("ZAddressOffsetMask: " +
>> ZAddressOffsetMask);
>> ```
>>
>> Can we remove L87?
>
> Haha. Yes, thanks for noticing this.
>
>>
>>
>>> However, that doesn't solve the bug, because there's another bug in
>>> the SA agent that truncates longs to ints. I created a bug report for
>>> that:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8195613
>>
>> Thanks!
>> I tried to use lookupLongConstant() to get const values in
>> zGlobals.hpp , but I couldn't.
>> I guess it caused by JDK-8195613.
>>
>> BTW, should we merge this SA change at first like JDK-8194737?
>>
>
> I've folded fixes for JDK-8195613 and JDK-8194737 into this patch and
> will push this to zgc/zgc, so that we don't have to wait for the
> upstream patches to get pushed.
>
>>
>> Others looks good to me.
>
> Thanks and thanks for the help!
>
> StefanK
>
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> On 2018/01/17 21:57, Stefan Karlsson wrote:
>>> Hi Yasumasa,
>>>
>>> On 2018-01-17 03:51, Yasumasa Suenaga wrote:
>>>> Hi Stefan,
>>>>
>>>> I uploaded new webrev:
>>>> http://cr.openjdk.java.net/~ysuenaga/z/sa-universe/webrev.03/
>>>
>>> I created a delta diff for your patch:
>>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/yasumasa_webrev.03_diff
>>>
>>>
>>>>
>>>> I removed dependencies to Linux specific code in it.
>>>
>>> Thanks!
>>>
>>> I've made some unifications, cleanups, and fixes to the patch:
>>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/webrev.04.delta
>>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/webrev.04
>>>
>>> Notably:
>>>
>>> 1) Got rid of the duplicated calculations of constants and instead
>>> read the values from the database.
>>>
>>> 2) There's bug in your code:
>>> http://cr.openjdk.java.net/~stefank/zgc/zSALoadBarriers/yasumasa_webrev.03_diff/src/hotspot/share/runtime/vmStructs.cpp.udiff.html
>>>
>>>
>>> + VM_LONG_CONSTANTS_Z(GENERATE_VM_INT_CONSTANT_ENTRY,
>>> + GENERATE_VM_INT_CONSTANT_WITH_VALUE_ENTRY)
>>>
>>> Should be using the _LONG_ versions.
>>>
>>> However, that doesn't solve the bug, because there's another bug in
>>> the SA agent that truncates longs to ints. I created a bug report for
>>> that:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8195613
>>>
>>> 3) There's a bug in the proposed ZAddress.address. If value is 0,
>>> then the orWithMask will throw an NPE. I've changed the code to
>>> manipulate the value, and as a last step convert it with
>>> ZOop.to_address.
>>>
>>> Thanks,
>>> StefanK
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>
>>>> 2018-01-16 20:00 GMT+09:00 Yasumasa Suenaga <yasuenag at gmail.com>:
>>>>>> I checked this patch with -XX:+ZUnmapBadViews on my Linux x64, it
>>>>>> works fine.
>>>>>
>>>>> Sorry, I forgot to run GC.run jcmd. I had a error.
>>>>> I will look it later.
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> 2018-01-16 19:57 GMT+09:00 Yasumasa Suenaga <yasuenag at gmail.com>:
>>>>>> Hi Stefan,
>>>>>>
>>>>>> I've restored your change and use VM.getOS() and getCPU() in new
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~ysuenaga/z/sa-universe/webrev.02/
>>>>>>
>>>>>> I checked this patch with -XX:+ZUnmapBadViews on my Linux x64, it
>>>>>> works fine.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2018-01-16 18:26 GMT+09:00 Stefan Karlsson
>>>>>> <stefan.karlsson at oracle.com>:
>>>>>>> Hi Yasumas,
>>>>>>>
>>>>>>> On 2018-01-16 08:14, Yasumasa Suenaga wrote:
>>>>>>>>
>>>>>>>> Hi Stefan,
>>>>>>>>
>>>>>>>>>> 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?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'll let Jini handle that bug.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>> I watch this bug.
>>>>>>>>
>>>>>>>>
>>>>>>>>> On SPARC we add a heap base when converting from "offsets" to
>>>>>>>>> "objects".
>>>>>>>>> See:
>>>>>>>>>
>>>>>>>>> http://hg.openjdk.java.net/zgc/zgc/file/89d447cff6c5/src/hotspot/os_cpu/solaris_sparc/zAddress_solaris_sparc.inline.hpp
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I think we can use getOS() and getCPU() in VM class:
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/zgc/zgc/file/89d447cff6c5/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/VM.java#l511
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> My first version of of ZCollectedHeap.oop_load_at passed an
>>>>>>>>> OopHandle to
>>>>>>>>> ZBarrier.weak_barrier, but I changed that, so my implementation of
>>>>>>>>> ZOop.to_address isn't needed there. However, it is still needed
>>>>>>>>> for
>>>>>>>>> ZCollectedHeap.oopAddressDescription:
>>>>>>>>>
>>>>>>>>> public String oopAddressDescription(OopHandle handle) {
>>>>>>>>> Address origOop = ZOop.to_address(handle);
>>>>>>>>> Address loadBarrieredOop =
>>>>>>>>> ZBarrier.weak_barrier(origOop);
>>>>>>>>>
>>>>>>>>> We either need to restore the previous implementation, or we
>>>>>>>>> need to fix
>>>>>>>>> oopAddressDescription.
>>>>>>>>
>>>>>>>>
>>>>>>>> OK, I will keep your original ZOop.java and related implementation.
>>>>>>>
>>>>>>>
>>>>>>> OK. We can revisit this later if we do decide to clean this up.
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>> I added these values to constant values in VMStructs.
>>>>>>>>>> Thus I removed ZGlobalsForVMStructs.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This is a bug, and causes my test to fail.
>>>>>>>>>
>>>>>>>>> The constants values in VMStructs are for constants in the C++
>>>>>>>>> code. Some
>>>>>>>>> of
>>>>>>>>> the values you changed are initialized during the VM
>>>>>>>>> initializations, and
>>>>>>>>> some are changed throughout the run of the VM.
>>>>>>>>
>>>>>>>>
>>>>>>>> I guess we can use HotSpotVMAddress as JVMCI:
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/zgc/zgc/file/89d447cff6c5/src/hotspot/share/jvmci/vmStructs_jvmci.cpp#l937
>>>>>>>>
>>>>>>>>
>>>>>>>> However this symbol is not exported.
>>>>>>>> We need to add `gHotSpotVMAddress` to vmStructs.hpp .
>>>>>>>>
>>>>>>>> BTW, do ZGC team have a plan to refactor zGlobals.hpp?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Currently, we don't have any plans to do that.
>>>>>>>
>>>>>>>> AFAICS HotSpot does not have global variables (not `const` value).
>>>>>>>> IMHO we can refactor zGlobals to the C++ class which extends
>>>>>>>> AllStatic.
>>>>>>>> We can use VMStructs directly if we do so.
>>>>>>>
>>>>>>>
>>>>>>> HotSpot actually does have global variables. You'll find some of
>>>>>>> them in
>>>>>>> globalDefinitions.hpp. For example: heapOopSize, MinObjAlignment,
>>>>>>> and
>>>>>>> OopEncodingHeapMax.
>>>>>>>
>>>>>>> The SA seems to duplicate the calculations of these values
>>>>>>> instead of
>>>>>>> reading the actual values from the process/core.
>>>>>>>
>>>>>>> I'm a bit reluctant to change the ZGC code style just to adapt to
>>>>>>> the SA
>>>>>>> agent. Currently, I think it's fine to have an extra layer in
>>>>>>> vmStructs_z
>>>>>>> until the SA agent gets support for global variables.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> StefanK
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Try to run with -XX:+ZUnmapBadViews. You will find any stray
>>>>>>>>> pointers
>>>>>>>>> faster
>>>>>>>>> with that flag.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>> After all concerns in above is resolved, I will test it.
>>>>>>>>
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> 2018-01-15 17:48 GMT+09:00 Stefan Karlsson
>>>>>>>> <stefan.karlsson at oracle.com>:
>>>>>>>>>
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> On 2018-01-13 04:39, Yasumasa Suenaga wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'll let Jini handle that bug.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> OK. Sounds like the right thing to do for true constants. More
>>>>>>>>> about this
>>>>>>>>> later.
>>>>>>>>>
>>>>>>>>>> But I've not remove address() from ZAddress.java because I'm
>>>>>>>>>> not sure
>>>>>>>>>> why
>>>>>>>>>> this method is needed.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On SPARC we add a heap base when converting from "offsets" to
>>>>>>>>> "objects".
>>>>>>>>> See:
>>>>>>>>>
>>>>>>>>> http://hg.openjdk.java.net/zgc/zgc/file/89d447cff6c5/src/hotspot/os_cpu/solaris_sparc/zAddress_solaris_sparc.inline.hpp
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> My version of ZOop.to_address converted a OopHandle into an
>>>>>>>>> non-OopHandle
>>>>>>>>> Address. Without it you get an UnsupportedOperationException
>>>>>>>>> when you try
>>>>>>>>> to
>>>>>>>>> do bit-manipulation on the OopHandle. From LinuxOopHandle:
>>>>>>>>> public Address addOffsetTo (long offset) throws
>>>>>>>>> UnsupportedOperationException {
>>>>>>>>> throw new UnsupportedOperationException("addOffsetTo not
>>>>>>>>> applicable
>>>>>>>>> to
>>>>>>>>> OopHandles (interior object pointers not allowed)");
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> My first version of of ZCollectedHeap.oop_load_at passed an
>>>>>>>>> OopHandle to
>>>>>>>>> ZBarrier.weak_barrier, but I changed that, so my implementation of
>>>>>>>>> ZOop.to_address isn't needed there. However, it is still needed
>>>>>>>>> for
>>>>>>>>> ZCollectedHeap.oopAddressDescription:
>>>>>>>>>
>>>>>>>>> public String oopAddressDescription(OopHandle handle) {
>>>>>>>>> Address origOop = ZOop.to_address(handle);
>>>>>>>>> Address loadBarrieredOop =
>>>>>>>>> ZBarrier.weak_barrier(origOop);
>>>>>>>>>
>>>>>>>>> We either need to restore the previous implementation, or we
>>>>>>>>> need to fix
>>>>>>>>> oopAddressDescription.
>>>>>>>>>
>>>>>>>>> My version of ZOop.to_address performed bit manipulations just
>>>>>>>>> to ensure
>>>>>>>>> that we never got a all-zero value. You have left the
>>>>>>>>> andWithMask(REMOVE_MSB_MASK), which isn't necessary when you
>>>>>>>>> didn't add
>>>>>>>>> the
>>>>>>>>> bit in the first place.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> OK.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This is a bug, and causes my test to fail.
>>>>>>>>>
>>>>>>>>> The constants values in VMStructs are for constants in the C++
>>>>>>>>> code. Some
>>>>>>>>> of
>>>>>>>>> the values you changed are initialized during the VM
>>>>>>>>> initializations, and
>>>>>>>>> some are changed throughout the run of the VM.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>> 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?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> You can use any program that starts a number of threads an
>>>>>>>>> continuously
>>>>>>>>> run
>>>>>>>>> GCs.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I tested `universe` and `jstack` and `jstack -v` on CLHSDB
>>>>>>>>>> with simple
>>>>>>>>>> Java app.
>>>>>>>>>> This change works fine on my Linux x64 box.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Try to run with -XX:+ZUnmapBadViews. You will find any stray
>>>>>>>>> pointers
>>>>>>>>> faster
>>>>>>>>> with that flag.
>>>>>>>>>
>>>>>>>>> This reproduces the problem:
>>>>>>>>>
>>>>>>>>> public class HelloSleep {
>>>>>>>>> public static void main(String... args) throws Exception {
>>>>>>>>> Thread.sleep(100000000000000L);
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> DIR=~/hg/z/build/release/jdk
>>>>>>>>> $DIR/bin/java -XX:+ZUnmapBadViews -XX:+UseZGC HelloSleep &
>>>>>>>>> $DIR/bin/jcmd $! GC.run
>>>>>>>>> echo -e "verbose true\nthreads" | $DIR/bin/jhsdb -J-ea hsdb
>>>>>>>>> --pid=$
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> StefanK
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
More information about the zgc-dev
mailing list