RFR: 8253180: ZGC: Implementation of JEP 376: ZGC: Concurrent Thread-Stack Processing [v7]
Roman Kennke
rkennke at openjdk.java.net
Tue Sep 29 12:52:05 UTC 2020
On Tue, 29 Sep 2020 10:02:16 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:
>> IMO it's ok to remove it.
>> However, it can be argued that is_in() should not check for 'in committed memory' but 'in reserved space for heap', IOW
>> 'is this a pointer into our heap memory region?'. Or maybe this would be CH::is_in_reserved() and we should change the
>> assert to that? Also, IMO we should not do anything that may trip barriers on oop-iterators or similar GC-only paths.
>
> Almost always, what you want to do, is to assert the validity of heap pointers, where they are expected to be valid
> (dereferenceable pointer to a not freed object). And then you actually do want to check that they are within the
> committed boundaries of the heap. Otherwise you have a bug. We don't want to make such reasonable assertions weaker,
> because we cater for asserts that want oops to look valid pretty much all the time, even when they are stale pointers
> from a previous cycle, pointing into freed memory (until it is fixed). And I would much rather cater for asserts with
> good underlying reasoning (the oop should actually be okay here or something is wrong), than to cater for asserts that
> seem to just have the wrong underlying reasoning, making everyone else suffer for it, and letting bugs slip past all
> the other asserts. The is_in_reserved check no longer exists. And I don't think I want to bring it back from the dead
> for this assert. Because I don't think the assert is right. The GCs already have their own verification code for roots
> (and the heap), that does this job, but much better. So I don't think it makes sense for the actual shared code oop
> iterator to go in and put constraints on what an invalid (not yet fixed up) stale pointer should and should not look
> like, for all GC implementations. Let the GC's own verification code take care of that. It has a much better idea about
> what a stale pointer should and should not look like. And I do agree that we should not use barriers in this kind of
> code path. I just didn't know what else to do here to make the assert happy. Sorry about that. Well now I know. I am
> going to delete it, unless anyone opposes that solution.
It's ok for me.
-------------
PR: https://git.openjdk.java.net/jdk/pull/296
More information about the serviceability-dev
mailing list