[8u] RFR Backport 8057003: Large reference arrays cause extremely long synchronization times
Hohensee, Paul
hohensee at amazon.com
Mon Jul 20 15:46:44 UTC 2020
Thanks, Liang. The issue was already tagged with jdk8u-fix-request, and I’ve added a ‘review approval’ comment.
Paul
From: Liang Mao <maoliang.ml at alibaba-inc.com>
Date: Monday, July 20, 2020 at 12:34 AM
To: "Hohensee, Paul" <hohensee at amazon.com>, 'jdk8u-dev' <jdk8u-dev at openjdk.java.net>
Subject: RE: [EXTERNAL] [8u] RFR Backport 8057003: Large reference arrays cause extremely long synchronization times
Hi Paul,
Here is the version with the comment change:
http://cr.openjdk.java.net/~ddong/liangmao/8057003/webrev.02/
Thank you for the sponsor.
Liang
From: Hohensee, Paul [mailto:hohensee at amazon.com]
Sent: 2020年7月17日 22:47
To: Liang Mao <maoliang.ml at alibaba-inc.com>; 'jdk8u-dev' <jdk8u-dev at openjdk.java.net>
Subject: RE: [8u] RFR Backport 8057003: Large reference arrays cause extremely long synchronization times
I’d change the oops_do() comment to
// G1 does its own checking
because the assert has a general G1 check (i.e., UseG1GC) rather than a specific one. All uses of oops_do() by G1 will have to do their own checks, not just the array slice case that your current comment says.
Other than that, lgtm. I’ll sponsor the patch and add a ‘reviewed’ comment to the issue.
Thanks,
Paul
From: Liang Mao <maoliang.ml at alibaba-inc.com<mailto:maoliang.ml at alibaba-inc.com>>
Date: Thursday, July 16, 2020 at 11:58 PM
To: "Hohensee, Paul" <hohensee at amazon.com<mailto:hohensee at amazon.com>>, 'jdk8u-dev' <jdk8u-dev at openjdk.java.net<mailto:jdk8u-dev at openjdk.java.net>>
Subject: RE: [8u] RFR Backport 8057003: Large reference arrays cause extremely long synchronization times
Hi Paul,
Thanks very much for your detailed review! I just upload the latest change.
Could you please help to push if you are ok with it?
http://cr.openjdk.java.net/~ddong/liangmao/8057003/webrev.01/
Thanks,
Liang
From: Hohensee, Paul [mailto:hohensee at amazon.com]
Sent: 2020年7月16日 22:30
To: Liang Mao <maoliang.ml at alibaba-inc.com<mailto:maoliang.ml at alibaba-inc.com>>; 'jdk8u-dev' <jdk8u-dev at openjdk.java.net<mailto:jdk8u-dev at openjdk.java.net>>
Subject: RE: [8u] RFR Backport 8057003: Large reference arrays cause extremely long synchronization times
Using #ifndef INCLUDE_ALL_GCS is also a hack. :)
I looked for uses of GenericTaskQueue::oops_do() in G1 and could find only one, in ConcurrentMark::verify_no_cset_oops(). In that context, it invokes VerifyNoCSetOopsClosure::do_object_work(), which is updated by the original patch. But, JDK 9 has two asserts rather than JDK 8’s single assert. The first JDK 9 assert is the G1-specific equivalent to the assert in TaskQueue::oops_do(). So, I'd add the first JDK 9 assert to your patch’s VerifyNoCSetOopsClosure::do_object_work(), vis
guarantee(G1CMObjArrayProcessor::is_array_slice(obj) || obj->is_oop(),
"Non-oop " PTR_FORMAT ", phase: %s, info: %d",
p2i(obj), _phase, _info);
and change the assert in TaskQueue::oops_do() to what you suggest (with a comment), vis
// G1 does its own checking
assert(UseG1GC || (*t)->is_oop_or_null(), "Not an oop or null");
Thanks,
Paul
On 7/15/20, 7:28 PM, "Liang Mao" <maoliang.ml at alibaba-inc.com<mailto:maoliang.ml at alibaba-inc.com>> wrote:
Hi Paul,
In order to partially iterate the object array, the task queue will not
only have oop but also "array slice" which is not oop. But inserting
G1 specific code into taskqueue.hpp as below seems a little bit hacking:
+ assert((*t)->is_g1_array_slice() || (*t)->is_oop_or_null(), "Not an oop or null");
In JDK9, the oops_do() is already replaced by a common general iterate()
which won't have that assertion. Do you think changing the assertion as
below is better?
-assert((*t)->is_oop_or_null(), "Not an oop or null");
+assert(UseG1GC || (*t)->is_oop_or_null(), "Not an oop or null");
Thanks,
Liang
> -----Original Message-----
> From: Hohensee, Paul [mailto:hohensee at amazon.com]
> Sent: 2020年7月16日 5:39
> To: Liang Mao <maoliang.ml at alibaba-inc.com<mailto:maoliang.ml at alibaba-inc.com>>; jdk8u-dev <jdk8u-
> dev at openjdk.java.net<mailto:dev at openjdk.java.net>>
> Subject: RE: [8u] RFR Backport 8057003: Large reference arrays cause extremely
> long synchronization times
>
> Lgtm, except, why you need to remove the assert from taskqueue.hpp?
> oops_do() doesn't seem to exist in 9, though I see an iterate() method that looks
> like it does the same thing in 9. INCLUDE_ALL_GCS is undefined only when the
> serial collector is the only collector being compiled into Hotspot, so with your
> change the assert only gets compiled for that case. Is a taskqueue ever
> instantiated for that case?
>
> Thanks,
> Paul
>
> On 7/6/20, 6:13 AM, "jdk8u-dev on behalf of Liang Mao" <jdk8u-dev-
> retn at openjdk.java.net<mailto:retn at openjdk.java.net> on behalf of maoliang.ml at alibaba-inc.com<mailto:maoliang.ml at alibaba-inc.com>> wrote:
>
> Hi,
>
> Could someone please help?
>
> Thanks,
> Liang
>
>
>
>
> ------------------------------------------------------------------
> From:MAO, Liang <maoliang.ml at alibaba-inc.com<mailto:maoliang.ml at alibaba-inc.com>>
> Send Time:2020 Jul. 2 (Thu.) 10:30
> To:jdk8u-dev <jdk8u-dev at openjdk.java.net<mailto:jdk8u-dev at openjdk.java.net>>
> Subject:[8u] RFR Backport 8057003: Large reference arrays cause extremely
> long synchronization times
>
> Hi,
>
> I'm requesting the backport of 8057003 into 8u. As the description in the bug,
> it's a serious G1 issue which leads to 10X longer concurrent mark time and
> hang
> the Java threads for seconds. We encounter the problem in several
> applications.
> Since G1 is widely used in 8u, we need to fix it.
>
> Original bug:
> https://bugs.openjdk.java.net/browse/JDK-8057003
> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/a67614dce6cd
>
> The JDK9 patch did not apply cleanly. Although it is not a small patch, I
> didn't change much except some assertions. One assertion in taskqueue.hpp
> is removed. It should be ok because it's no longer there as well since JDK9.
>
> 8u webrev:
> http://cr.openjdk.java.net/~ddong/liangmao/8057003/webrev.00/
>
> Testing:
> gc/g1, specjbb2015 with fastdebug
>
> Could someone please have a look and sponsor?
>
> Thanks,
> Liang
>
>
More information about the jdk8u-dev
mailing list