RFR: 8306582: Remove MetaspaceShared::exit_after_static_dump() [v3]

[Ioi Lam]

On Thu, 27 Jul 2023 13:40:56 GMT, Matias Saavedra Silva <matsaave at 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