RFR: 8213623: ZGC: Let heap iteration walk all roots

[Per Liden]

Hi Kim,

On 11/12/18 10:49 AM, Kim Barrett wrote:
[...]
> It seems I misunderstood how this proposed change relates to a slack
> discussion we had a couple of weeks ago.
> 
> I think there was consensus from that discussion that existing heap
> walkers should be using AS_NO_KEEPALIVE.  The proposed change is
> addressing one case of that, which is good in so far as it goes.
> 
> ZGC implements JVMTI IterateThroughHeap via something that is similar
> to but different from JVMTI's root-based FollowReferences, because the
> ZGC heap is not reliably parseable in a way that would support the
> kind of heap iteration done by other collectors.
> 
> Based on that earlier discussion, I think JVMTI's FollowReferences
> should be including off-heap weak references like jweaks in its root
> set, for reasons that are similar to what was given for including them
> in ZGCs IterateThroughHeap implementation with this change.  I was
> confused and thought this change was affecting FollowReferences for
> ZGC.

As you noted, this patch doesn't affect the semantics of 
FollowReferences. But I agree that the sub-set of roots followed by 
FollowReferences (where initial_object is null) can indeed be 
questioned/discussed.

> 
> But that leads me to question why we need the code that is being
> changed here at all?  Why not fix FollowReferences, let ZGC use it as
> its implementation of IterateThroughHeap, and delete some code?  Am I
> missing something that makes that not work?

Better code sharing in this area is likely desirable. I'm generally 
uncomfortable with having the JVMTI code do this walking, instead of 
delegating that to some shared GC code (which GCs can override, or 
override parts of it when required). We've discussed a GC API like in 
the context of JFR, which have a very similar need in their 
leak-profiler. But doing something like that would be a fairly big 
undertaking.

Also note that today the ZHeapIterator is used for more than just the 
JVMTI stuff, like when doing heap verification, heap dumps, etc.

> 
> I think the change is okay, in that it does what it sets out to do.  I
> think we might be able to do better, but I'm okay with doing so later.
> 

Ok, thanks for reviewing.

cheers,
Per