RFR: 8308603: Removing do_pending_ref/enclosing_ref from MetaspaceClosure

[Ioi Lam]

On Tue, 23 May 2023 11:49:41 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> **The problem**
>> 
>> MetaspaceClosure is used to traverse all *pointers* in the metaspace objects. It works recursively. A pointer is represented by the `MetaspaceClosure::Ref` type. For example, if you have
>> 
>> 
>> InstanceKlass* a = ..., *b = ...;
>> a->_super = b;
>> 
>> 
>> When we are traversing the pointer `a->_super`, we create a `Ref` that records the address of `b` and `&a->super`. However, we currently don't remember the address of `a` in the `Ref`. As a result, when processing the `Ref` inside a `push(Ref* ref)` function, we need to call `MetaspaceClosure::enclousing_ref()` to get to `a`, so that we can mark pointers inside it.
>> 
>> The reason for`do_pending_ref` is even more convoluted. Please see [JDK-8308603](https://bugs.openjdk.org/browse/JDK-8308603) for more details.
>> 
>> **The fix**
>> 
>> The code can be much more readable if we simply remember  `a` in the `Ref`.
>
> src/hotspot/share/cds/archiveBuilder.cpp line 452:
> 
>> 450: 
>> 451:   // We are dealing with 3 addresses:
>> 452:   // address o    = ref->obj(): We have found an object whose address is o.
> 
> Are 'obj' oops ?  We almost always refer to oops as obj but not other things.  Is 'object' an oop?  This operates on metadata right? I realize this terminology is pre-existing.  I suggest changing obj and object in comments at least in the new code to either metadata() or ptr() even, metaptr() ?   Unless this does move oops. I don't know this code.

All the "objects" in the code related to MetaspaceClosure refer to instances of MetaspaceObj. This code rarely interacts with code that work on oops, so there isn't much confusion in practice.

I'd rather not change part of the code or comments to use a different name than "object". I think that will be even more confusing, causing the reader to think that "metadata", for example, is materially different than "object".

Also, "metadata" is unfortunately not a good name, because not every MetaspaceObj is a Metadata.

If renaming is desirable, that should be done in a separate RFE.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14093#discussion_r1202974209