RFR(XS) 8230674 Heap dumps should exclude dormant CDS archived objects

[Jiangli Zhou]

Just to answer David's questions. Those are all very good questions!

A dormant object is an unreachable object in the archived Java heap
region. At the moment when the VM mapps in the 'Open' archive heap
regions, all objects within the regions are considered as dormant
objects. The state of an archived object changes when its reachability
changes. Any archived Java object that is reachable by a live
(non-dormant) object is effectively a non-dormant/live object.

An archived object state is changed explicitly when it is 'installed'.
HeapShared::materialize_archived_object() is called for that
particular object to make GC aware about the object. For example, when
a shared class is loaded, the corresponding archived mirror object is
'installed' in the shared klass. That's when the mirror object becomes
alive. Another example is an archived object for static field value
and all reachable objects from it. When the VM 'installs' an archived
static field value back to the field, the object becomes alive
explicitly. All reachable objects via the entry object also become
non-dormant/alive implicitly.

Please see more info in
https://wiki.openjdk.java.net/display/HotSpot/Caching+Java+Heap+Objects.

Will send review separately.

Best,
Jiangli

On Thu, Sep 5, 2019 at 11:12 PM David Holmes <david.holmes at oracle.com> wrote:
>
> On 6/09/2019 1:39 pm, Ioi Lam wrote:
> > On 9/5/19 8:18 PM, David Holmes wrote:
> >> Hi Ioi,
> >>
> >> On 6/09/2019 12:27 pm, Ioi Lam wrote:
> >>> https://bugs.openjdk.java.net/browse/JDK-8230674
> >>> http://cr.openjdk.java.net/~iklam/jdk14/8230674-heap-dump-exclude-dormant-oops.v01
> >>>
> >>>
> >>> Please review this small fix:
> >>>
> >>> When CDS is in use, archived objects are memory-mapped into the heap
> >>> (currently G1GC only). These objects are partitioned into
> >>> "subgraphs". Some of these subgraphs may not be loaded (e.g., those
> >>> related to jdk.internal.math.FDBigInteger) at the time a heap dump is
> >>> requested. >
> >>> When a subgraph is not loaded, some of the objects in this subgraph
> >>> may belong to a class that's not yet loaded.
> >>>
> >>> The bug happens when such an "dormant" object is dumped, but its class
> >>> is not dumped because the class is not in the system dictionary.
> >>>
> >>> There is already code in DumperSupport::dump_instance() that tries to
> >>> handle dormant objects, but it needs to be extended to cover arrays,
> >>> as well as and references from non-dormant object/arrays to dormant
> >>> ones.
> >>
> >> I have to confess I did not pay any attention to the CDS archived
> >> objects work, so I don't have a firm grasp of how you have implemented
> >> things. But I'm wondering how can you have a reference to a dormant
> >> object from a non-dormant one? Shouldn't the act of becoming
> >> non-dormant automatically cause the subgraph from that object to also
> >> become non-dormant? Or do you have "read barriers" to perform the
> >> changes on demand?
> >>
> >
> > Hi David,
> >
> > Thanks for the review.
> >
> > The dormant objects are not reachable via the GC roots. They become
> > non-dormant via explicit calls to JVM_InitializeFromArchive, after which
> > they become reachable via the static fields of loaded classes.
>
> Right, so is there a distinction between non-dormant and reachable at
> the time an object becomes non-dormant? I'm still unclear how a drmant
> array becomes non-dormant but still contains elements that refer to
> dormant objects.
>
> > The only issue here is heap dump is done by scanning all objects in the
> > heap, including unreachable ones
> >
> >    HeapObjectDumper obj_dumper(this, writer());
> >    Universe::heap()->safe_object_iterate(&obj_dumper);
> >
> > that's how these dormant objects are discovered during heap dump.
> >
> >> That aside the code changes seem reasonable, you moved the check out
> >> of DumperSupport::dump_instance and into the higher-level
> >> HeapObjectDumper::do_object so that it catches instances and arrays,
> >> plus you added a check for array elements.
> >>
> >
> > I am debating whether I should put the masking code in here:
> >
> > void DumpWriter::write_objectID(oop o) {
> >    o = mask_dormant_archived_object(o);  /// <---- add
> >    address a = (address)o;
> > #ifdef _LP64
> >    write_u8((u8)a);
> > #else
> >    write_u4((u4)a);
> > #endif
> > }
> >
> >
> > That way, even if a dormant object (unintentionally) becomes reachable
> > via the GC roots, we won't write an invalid reference to it (the object
> > "body" will not be written, so the ID will not point to anything valid).
> >
> > But this seems a little too aggressive to me. What do you think?
>
> It does seem a little aggressive as it seems to introduce the dormancy
> check into a lot of places that don't need it. But as I said I don't
> know this code so I'm really not the right person to ask.
>
> Cheers,
> David
> -----
>
> > Thanks
> > - Ioi
> >