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.
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?
Thanks
- Ioi
