On Thu, 27 Jul 2023 13:40:56 GMT, Matias Saavedra Silva <matsa...@openjdk.org> wrote:
>> Currently we exit the VM after static dumping with >> `MetaspaceShared::exit_after_static_dump()`. >> >> >> // We have finished dumping the static archive. At this point, there may be >> pending VM >> // operations. We have changed some global states (such as >> vmClasses::_klasses) that >> // may cause these VM operations to fail. For safety, forget these >> operations and >> // exit the VM directly. >> void MetaspaceShared::exit_after_static_dump() { >> os::_exit(0); >> } >> >> >> As the comment suggests, the VM state is altered when preparing and >> performing the static dump, so this change aims to prevent these state >> changes so the VM can exit normally after the static dump completes. There >> are three major aspects to this change: >> 1. Since the resolved references array in the Constant Pool is altered when >> preparing for a static dump, a "scratch copy" is created and archived >> instead >> 2. Symbols are sorted by address and have their hash recalculated. Similarly >> to point 1, the copies of the symbols that are to be archived have their >> hashes updated as opposed to the originals. >> 3. The handling of -Xshare:dump during argument parsing such that the VM can >> continue and exit normally with an exit code of 0. >> >> Verified with tier 1-9 tests. > > Matias Saavedra Silva has updated the pull request incrementally with one > additional commit since the last revision: > > Refactored KlassToOopHandleTable, Ioi and Alan comments Looks good. Just a couple of nits. src/hotspot/share/cds/heapShared.cpp line 331: > 329: mtClassShared> { > 330: public: > 331: oop get_oop(MetaspaceObj* obj) { The word `obj` may be confusing here as it may look like an oop. I think it's better to change it to `ptr`. A bit of background: CDS copies two types of objects: oops (archiveHeapWriter.cpp and heapShared.cpp) and MetsapceObjs (in archiveBuilder.cpp) In the former two files, the convention is to use `obj` to refer to oops, and `ptr` to refer to MetsapceObjs. In archiveBuilder.cpp, since we never use a mix of oops and MetsapceObjs, we use `obj` to refer to MetsapceObjs. This is mostly for historical purposes, and we might some day change it to use `ptr` to be more consistent. src/hotspot/share/oops/constantPool.cpp line 226: > 224: // Handle scratch_handle(THREAD, scratch_references); > 225: //HeapShared::add_scratch_resolved_references(this, > loader_data->add_handle(scratch_handle)); > 226: HeapShared::add_scratch_resolved_references(this, > scratch_references); Comments should be removed? ------------- Marked as reviewed by iklam (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14879#pullrequestreview-1550470579 PR Review Comment: https://git.openjdk.org/jdk/pull/14879#discussion_r1276637489 PR Review Comment: https://git.openjdk.org/jdk/pull/14879#discussion_r1276639485