RFR: 8230566: ZGC: Don't substitute klass pointer during array clearing
Per Liden
per.liden at oracle.com
Wed Sep 4 17:15:20 UTC 2019
Hi Erik,
On 9/4/19 5:12 PM, Erik Österlund wrote:
> Hi Per,
>
> In src/hotspot/share/gc/z/zHeapIterator.cpp the only change made here
> is to mess up the indentation:
>
> 195 // Push roots to visit
> 196 push_roots<ZRootsIterator, false /* Concurrent */, false /* Weak */>();
> 197 push_roots<ZConcurrentRootsIteratorClaimOther, true /* Concurrent */, false /* Weak */>();
>
> Should probably change that back.
Will fix!
>
> One possible solution to the problem of passing around all this context information in the templates,
> is to wrap them in a template config type, like this:
>
> template <boolfollow, bool finalizable, bool publish>
> struct ZMarkConfig {
> static const bool _follow = follow;
> static const bool _finalizable = finalizable;
> static const bool _publish = publish;
> };
>
> Then you can just pass around the ZMarkConfig in the templates all over the place, instead of the 3 (at this
> moment) bools. That way, adding and removing bools becomes easier in the future; no need to change the passing
> around code to pass more things around. Only the producer and consumer of the flags need changing.
>
> However, I'd propose we do that in a follow-up RFE so we can fix the bug faster. Having said that, looks good,
> and don't need a new webrev.
I'll prototype and see how it turns out. If it seems to be worth doing
I'll file a separate RFE.
Thanks for reviewing!
cheers,
Per
>
> Thanks,
> /Erik
>
> On 2019-09-04 14:02, Per Liden wrote:
>> When allocating large object arrays, ZObjArrayAllocator instantiates a
>> non-cleared array of longs and later installs the correct klass
>> pointer once the array has been cleared. While this might work it's
>> also error prone. For example, there are at least two asserts() that
>> will re-loaded the klass pointer and fail. It's also easy for someone
>> in the future to make the innocent mistake of re-loading the klass
>> pointer in some sensitive path, which would lead to new problems. We
>> can avoid all these problems by not substituting with the klass
>> pointer, and instead have another mechanism to tell GC marker threads
>> to not follow the elements in not-yet-cleared arrays.
>>
>> This patch adds a new bit to ZMarkStackEntry, to signal to marking
>> threads that the object array should not be followed, just mark it as
>> live and visit its klass.
>>
>> The patch is large-ish because of the need to propagate the "follow"
>> flag down through a few of layers. We might want to think about
>> converting the flags (follow/finalizable/publish) into a single
>> ZBarrierFlags-thing (perhaps similar to DecoratorSet) to reduce the
>> noise and make it easier to add/adjust/remove flags in the future. But
>> such an enhancements could come later.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8230566
>> Webrev: http://cr.openjdk.java.net/~pliden/8230566/webrev.0
>>
>> Testing: tier1-6 on linux + ad hoc runs of tests that provoked the
>> problem
>>
>> /Per
>
More information about the hotspot-gc-dev
mailing list