RFR: 8253180: ZGC: Implementation of JEP 376: ZGC: Concurrent Thread-Stack Processing [v7]

[Erik Österlund]

On Mon, 28 Sep 2020 17:20:49 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> We call frame::oops_do from the GC to make bad oops good. But the oops_do logic assers the oop is good *before* the
>> closure is applied. Every time I do something, I run i to issues with this assert. It seems like an invalid assumption
>> to me, that oops are meant to always be good, even before they are fixed. I vote for removing this assert. It has
>> caused a lot of pain. Any takers?
>
> I think the previous assert was intentionally weaker: `is_in` checks that argument points to a committed area in the
> heap, which can include the oops not yet fixed. It does not check if oop is valid as far as GC is concerned. So maybe
> reverting `NativeAccess::oop_load` is enough?

It was checking is_in_or_null before and after. Our is_in checks that it is in committed memory, and that the pointer
is not stale w.r.t. color, as that is very likely to be a bug. We reluctantly made it relaxed for our safepoints that
flip colors. I think it was once again for this very same usual suspect assert. But now that this is concurrent, its
memory can get concurrently freed.

I think this is the 7th time I try to make this assert happy. I run into it all the time. IMO, the underlying
assumption of the assert, is wrong. This is an iterator used by GC to fix totally invalid stale pointers into valid
pointers. You really can't expect that they are valid before fixing them. That just happened to be true for other GCs.
It makes little sense to me. That is why my preferred solution is to remove the assert. I would not miss it.

We have had similar issues with the oop_iterate framework asserting oops are valid before the closure is applied. At
least there, it is possible to opt out...

Would anyone miss the assert?

-------------

PR: https://git.openjdk.java.net/jdk/pull/296