RFR:8206009: Move java heap object archiving code to heapShared.hpp and heapShared.cpp (original) (raw)

Jiangli Zhou jiangli.zhou at oracle.com
Tue Oct 9 05:29:27 UTC 2018

On 10/5/18 9:17 AM, Ioi Lam wrote:

Looks good. Thanks!

Thanks. I've merged with the subgraph info hashtable change that you integrated today. Universe::serialize() is shared by both dump time and runtime. To be more precise, I also added DumpSharedSpaces to the new assert:

#ifdef ASSERT   if (DumpSharedSpaces && !HeapShared::is_heap_object_archiving_allowed()) {     assert(_int_mirror == NULL    && _float_mirror == NULL &&            _double_mirror == NULL && _byte_mirror == NULL &&            _bool_mirror == NULL   && _char_mirror == NULL &&            _long_mirror == NULL   && _short_mirror == NULL &&            _void_mirror == NULL, "mirrors should be NULL");   } #endif

I'll integrate after re-running the tests.

Thanks, Jiangli

- Ioi

On 10/4/18 10:55 PM, Jiangli Zhou wrote: On 10/4/18 8:16 PM, Ioi Lam wrote:

On 10/4/18 11:00 AM, Jiangli Zhou wrote: Hi Ioi, Thanks for the review. On 10/4/18 8:51 AM, Ioi Lam wrote: Hi Jiangli, The fix looks good. #if INCLUDECDSJAVAHEAP // The mirrors are NULL if HeapShared::isheapobjectarchivingallowed // is false. f->dooop(&intmirror); f->dooop(&floatmirror); Maybe the comment should be turned into an assert? There are existing asserts in Universe::initializebasictypemirrors() already that verify the primitive type mirrors are not NULL when HeapShared::isheapobjectarchivingallowed is true. if (UseSharedSpaces && HeapShared::openarchiveheapregionmapped() && intmirror != NULL) { assert(HeapShared::isheapobjectarchivingallowed(), "Sanity"); assert(floatmirror != NULL && doublemirror != NULL && bytemirror  != NULL && bytemirror != NULL && boolmirror  != NULL && charmirror != NULL && longmirror  != NULL && shortmirror != NULL && voidmirror  != NULL, "Sanity"); But these asserts are the opposite of what the comment says: "The mirrors are NULL if HeapShared::isheapobjectarchivingallowed is false." The existing asserts are "A implies B". I think it's better to also add "not A implies not B". I replaced with the following assert in Universe::serialize(): #ifdef ASSERT if (!HeapShared::isheapobjectarchivingallowed()) { assert(intmirror == NULL    && floatmirror == NULL && doublemirror == NULL && bytemirror == NULL  && boolmirror == NULL   && charmirror == NULL  && longmirror == NULL   && shortmirror == NULL && voidmirror == NULL, "mirrors should be NULL"); } #endif Here is the updated webrev: http://cr.openjdk.java.net/~jiangli/8206009/webrev.02/src/hotspot/share/memory/universe.cpp.frames.html Thanks, Jiangli Thanks - Ioi

Also, have you tested with configure --disable-precompiled-headers? Build with --disable-precompiled-headers works without failure on linux-x64. I will queue my change after your subgraph info hash table change for 8210388 and resolve conflicts. Thanks, Jiangli

Thanks - Ioi

On 10/3/18 3:23 PM, Jiangli Zhou wrote: Please review the restructuring and cleanup of java heap object archiving code. The java object archiving code has grown in the past year and metaspaceShared.* files are not the suitable place. The restructuring and cleanup include: - Moved java heap object archiving implementation from metaspaceShared.* to heapShared.*. - Various isarchiveobject() APIs are renamed to isarchivedobject() for naming consistency: - Renamed MetaspaceShared::isarchiveobject() to HeapShared::isarchivedobject(). - Renamed oopDesc::isarchiveobject() to oopDesc::isarchivedobject(). - Renamed G1ArchiveAllocator::isarchiveobject() to G1ArchiveAllocator::isarchivedobject(). - Changed to use G1ArchiveAllocator::isarchivedobject() in G1CollectedHeap::materializearchivedobject(). Removed #include "memory/metaspaceShared.inline.hpp” from g1CollectedHeap.cpp. - Renamed HeapShared::archivestaticfields() to HeapShared::archiveobjectsubgraphs(). webrev: http://cr.openjdk.java.net/~jiangli/8206009/webrev.00/ RFE: https://bugs.openjdk.java.net/browse/JDK-8206009 Tested with tier1-tier3. Tier4 and tier5 are in progress. Thanks, Jiangli

More information about the hotspot-runtime-dev mailing list