RFR: 8263421: Module image file is opened twice during VM startup [v2]

[David Holmes]

On Wed, 21 Apr 2021 18:51:02 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

>> Please review this fix for JDK-8263421.  The fix stores the JImageFile* in a static that gets initialized when lookup_vm_options() opens the module image file.  The static JimageFIle* is then used by setup_bootstrap_search_path_impl() when creating the ClassPathImageEntry for the module image file, avoiding having to re-open the module image file.
>> 
>> This fix moved the creation of the ClassPathImageEntry for the module image file from create_class_path_entry() to setup_bootstrap_search_path_impl() because create_class_path_entry() may try to do this multiple times, but the ClassPathImageEntry for the module image file is only actually created once.
>> 
>> The fix also removes some unused code related to module image file processing.
>> 
>> The changes were tested with Mach5 tiers 1 and 2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-
>> 5 on Linux x64.
>> 
>> Thanks, Harold
>
> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add jimage_non_null(), modify destructor

Hi Harold,

Generally this looks okay. A couple of minor nits.

Thanks,
David

src/hotspot/share/classfile/classLoader.cpp line 112:

> 110: 
> 111: // JimageFile pointer, or null if exploded JDK build.
> 112: static JImageFile*                     JImage_File            = NULL;

Naming style looks odd here - JImage_file ? _jimage_file ?

src/hotspot/share/classfile/classLoader.cpp line 360:

> 358: void ClassPathImageEntry::set_jimage(JImageFile* jimage) {
> 359:   JImage_File = jimage;
> 360: }

This setter seems overkill, especially when it is only used to clear the field, and there is no setter used when giving the field a real value.

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

Marked as reviewed by dholmes (Reviewer).

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