Hi Zhengyu, Can’t your barriers just perform a NULL check on the forwardee instead? forwardee() == NULL never means forwarded, does it? Both JVMTI and JFR just ”mark” the markWord, leaving its forwardee == NULL.
That way you can solve the issue in the backend instead, and we don’t need to do anything about JFR either. Or did I miss something? Thanks, /Erik > On 24 Feb 2020, at 17:49, Zhengyu Gu <z...@redhat.com> wrote: > > Hi all, > > Updated according to your comments: > http://cr.openjdk.java.net/~zgu/JDK-8238633/webrev.02/ > > I modified vmTestbase/nsk/jvmti/unit/heap/HeapWalkTests/TestDescription.java > test [1] to walk 300K objects. > > Without patch: > Time: 987431 nsecs > Time: 1135390 nsecs > Time: 1142519 nsecs > Time: 962816 nsecs > Time: 1015958 nsecs > > Avg: 1048822 nsecs > > With patch: > 1105015 nsecs > 1142425 nsecs > 968057 nsecs > 1383838 nsecs > 1079885 nsecs > > Avg: 1135844 nsecs > > So, it shows about 8% performance hit. > > Thanks, > > -Zhengyu > > [1] http://cr.openjdk.java.net/~zgu/JDK-8238633/test/webrev.00/ > > > > > >> On 2/21/20 8:01 AM, coleen.phillim...@oracle.com wrote: >> Adding serviceability-dev back. >> Coleen >>> On 2/21/20 7:59 AM, coleen.phillim...@oracle.com wrote: >>> >>> Hi, I had a quick look at this, minus the shenandoah code. >>> >>> http://cr.openjdk.java.net/~zgu/JDK-8238633/webrev.01/src/hotspot/share/gc/shared/objectMarker.hpp.html >>> >>> >>> I think this file could have forward declarations of GrowableArray and I >>> didn't see a need for the markWord.hpp include. >>> >>> This change on the whole looks good to me. >>> >>> Coleen >>> >>> On 2/21/20 5:23 AM, Stefan Karlsson wrote: >>>> Hi Zhengyu, >>>> >>>> On 2020-02-17 15:51, Zhengyu Gu wrote: >>>>> Hi Stefan, >>>>> >>>>> Thanks for the review and suggestions, updated accordingly: >>>>> >>>>> http://cr.openjdk.java.net/~zgu/JDK-8238633/webrev.01/ >>>> >>>> Thanks for moving the code. I think this looks good. >>>> >>>> If you're up for it, I have a couple of style change suggestions: >>>> >>>> 1) ObjectMarker uses two verbs to describe the same thing: "mark" and >>>> "visit". I propose that we only use "mark" in ObjectMarker and leave the >>>> usage of "visited" to the Jvmti code. >>>> >>>> 2) Some updates to odd whitespaces >>>> >>>> 3) Using forward declarations in Shenandoah code. >>>> >>>> I've bundled those changes into webrevs: >>>> >>>> https://cr.openjdk.java.net/~stefank/8238633/webrev.01.delta >>>> https://cr.openjdk.java.net/~stefank/8238633/webrev.01 >>>> >>>> Regarding performance testing, the HeapWalkTests you used seems to use a >>>> very small heap. I think it would be good to redo the measurements on a >>>> larger heap. Could you take the HeapWalkTest and add a few GBs of small, >>>> linked objects? >>>> >>>> Thank, >>>> StefanK >>>>> >>>>>> >>>>>> --- >>>>>> Previously, the calls to 'mark' and 'visited' were inlineable, but now >>>>>> every GC has to take a virtual call when marking the objects. My guess >>>>>> is that this code is slow anyway, and that it doesn't matter too much, >>>>>> but did you measure the effect of that change with, for example, G1? >>>>>> >>>>> I did rough measurement, timing >>>>> vmTestbase/nsk/jvmti/unit/heap/HeapWalkTests/TestDescription.java test. >>>>> >>>>> If you know any tests/benchmarks I should measure, please let me know. >>>>> >>>>> Thanks, >>>>> >>>>> -Zhengyu >>>>> >>>>> >>>>>> Thanks, >>>>>> StefanK >>>>>> >>>>>>> Test: >>>>>>> hotspot_gc >>>>>>> vmTestbase_nsk_jdi >>>>>>> vmTestbase_nsk_jvmti >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> -Zhengyu >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >