RFR (S) 8227370 Remove SharedPathsMiscInfo

Jiangli Zhou jianglizhou at google.com
Tue Aug 27 00:56:53 UTC 2019


Nice cleanup! Just some minor issues/suggestions below.

- src/hotspot/share/classfile/classLoaderExt.cpp

 213         trace_class_path("library = ", libname); 214         if
(!ClassLoader::update_class_path_entry_list(libname, true, false, true
/* from_class_path_attr */)) { 215
FileMapInfo::record_non_existent_class_path_entry(libname); 216
 }

It might be useful to do trace_class_path() after
update_class_path_entry_list,  so the trace message can indicate if
the directory exists or not.

- src/hotspot/share/memory/filemap.cpp

 843 bool FileMapInfo::validate_non_existent_class_paths() const { 844
  assert(UseSharedSpaces, "runtime only"); 845   for (int i =
_header->_app_module_paths_start_index + _header->_num_module_paths;
846        i < get_number_of_shared_paths(); 847        i++) { 848
SharedClassPathEntry* ent = shared_path(i); 849     if
(!ent->check_non_existent()) { 850       fail_continue("file must not
exist: %s", ent->name()); 851       return false; 852     } 853   }
854   return true;

If check_non_existent() returns false, it could still use the archived
JDK classes at runtime and only invalidate the archived application
classes. So we don't lose all performance benefit.

 160   // All of these entries must be JAR files. The dumping process
would fail if a non-empty
 161   // directory was specified in the classpaths. If an empty
directory was specified

 162   // it is checked by the _paths_misc_info as described above.

 228   char* _paths_misc_info;

Could you please cleanup above _paths_misc_info references?

Thanks!

Jiangli


On Fri, Aug 23, 2019 at 4:56 PM Ioi Lam <ioi.lam at oracle.com> wrote:

> Hi Jiangli,
>
> Thanks for the info. I have added code and test for non-existent files
> specified in Class-Path:.
>
>
> http://cr.openjdk.java.net/~iklam/jdk14/8227370_remove_shared_misc_info.v02/
>
> I also consolidated some of the duplicated code in
> FileMapInfo::add_shared_classpaths().
>
> Thanks
> - Ioi
>
>
> On 8/23/19 8:47 AM, Jiangli Zhou wrote:
>
> Hi,
>
> It's not a complete code review yet. One good point that Calvin brought up
> during the code review email discussion for the previous related change was
> that runtime needs to check the non-existing JAR entries specified via the
> Class-Path header in manifest. Could you please verify those cases are
> handled properly with the change? Thanks.
>
> Regards,
> Jiangli
>
> On Thu, Aug 22, 2019 at 8:28 AM Ioi Lam <ioi.lam at oracle.com> wrote:
>
>> https://bugs.openjdk.java.net/browse/JDK-8227370
>>
>> http://cr.openjdk.java.net/~iklam/jdk14/8227370_remove_shared_misc_info.v01/
>>
>> For the 3 types of items stored in SharedPathsMiscInfo
>>
>> BOOT_PATH/APP_PATH:
>>
>> These are used only for tracing. I reimplemented the tracing
>> functionality in FileMapInfo::log_paths()
>>
>> NON_EXIST:
>>
>> It turns out we don't need to keep track of non-existent paths from the
>> dump-time path.  We can simply eliminate all the non-existent paths from
>> both the dump-time and runtime paths, and check that the remaining files
>> match. E.g.,
>>
>> dump: -cp a.jar:NE1:NE2:b.jar
>> run 1: -cp NE3:a.jar:NE4:b.jar
>> run 2: -cp x.jar:NE4:b.jar
>>
>> after elimination:
>>
>> dump = a.jar:b.jar
>> run 1 = a.jar:b.jar -> matched
>> run 2 = x.jar:b.jar -> mismatched
>>
>> The dump-time elimination was already done in
>> ClassLoader::update_class_path_entry_list().
>> The run-time elimination was already done in
>> FileMapInfo::create_path_array().
>>
>> So essentially I just tied things together and wrote a few more test cases
>> in NonExistClasspath.java
>>
>>
>> Testing: hs-tier{1,2,3}
>>
>>
>


More information about the hotspot-runtime-dev mailing list