RFR: 8273152: Refactor CDS FileMapHeader loading code [v4]

[Ioi Lam]

On Mon, 4 Oct 2021 17:50:27 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Please review,
>>   Refactor fundamental CDS FileMapHeader code for reliable reading of basic info from shared archive.
>>   With the change, it makes it possible to read an archive generated by different version of hotspot. Also it is possible to automatically generate a CDS archive If the archive supplied is not readable or fails to pass the check.
>> 
>>   Tests: tier1-4
>>    jtreg on sa.
>> 
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed comment

Changes requested by iklam (Reviewer).

src/hotspot/share/cds/filemap.cpp line 173:

> 171:   memset((void*)this, 0, sizeof(FileMapInfo));
> 172:   _is_static = is_static;
> 173:   size_t c_header_size;

FileMapInfo::FileMapInfo() is called in two places:

(1) When reading an archive -- _header should not be initialized here. It should be done inside FileMapInfo::init_from_file.

(2) When writing an archive -- I think we should move the _header initialization to FileMapInfo::populate_header().

src/hotspot/share/cds/filemap.cpp line 197:

> 195:   _header->set_base_archive_path_offset(0);
> 196:   _header->set_base_archive_name_size(0);
> 197:   _header->set_header_size((unsigned int)header_size);

The above 3 lines can be replaced by lines 203 .. 205, and 203 .. 205 can be deleted.

src/hotspot/share/cds/filemap.cpp line 200:

> 198:   _header->set_has_platform_or_app_classes(true);
> 199:   if (!is_static) {
> 200:     _header->set_base_archive_is_default(FLAG_IS_DEFAULT(SharedArchiveFile));

There's no need for a separate _base_archive_is_default field. We are using the base archive if base_archive_name_size == 0.

src/hotspot/share/cds/filemap.cpp line 1057:

> 1055:   GenericCDSFileMapHeader* header = (GenericCDSFileMapHeader*)os::malloc(sz, mtInternal);
> 1056:   memset((void*)header, 0, sz);
> 1057:   size_t n = os::read(fd, (void*)header, (unsigned int)sz);

There are currently 3 different places where we read the GenericCDSFileMapHeader, find out the size, and then read the real header.

- FileMapInfo::get_base_archive_name_from_header
- FileMapInfo::check_archive
- FileMapInfo::init_from_file

I think these should be consolidated into a single function.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5768