On Thu, 27 Jul 2023 13:40:56 GMT, Matias Saavedra Silva <[email protected]>
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
