Hi Erik,
On 2/24/20 12:04 PM, Erik Österlund wrote:
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?
You are right, this is a much simple solution. But the concern is that,
resolve_forward() is the most used barrier, additional null check is
undesirable.
After offline chat with my colleagues, we realize that it may be ok. As
JVMTI/JFR heap walk happens at safepoints, we really don't have to add
the null check in regular barrier. Instead, force GC to use different
version of resolve_forward with null check.
Let me protocol this alternative, will get back you soon.
Thank,
-Zhengyu
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
