RFR (S) 8227370 Remove SharedPathsMiscInfo

[Jiangli Zhou]

Looks good.

Nit: 'we we'
157   // during dumping. At run time, we we validate these entries
according to their

No new webrev is needed for me.

Thanks,
Jiangli

On Tue, Aug 27, 2019 at 10:57 AM Ioi Lam <ioi.lam at oracle.com> wrote:
>
> Hi Jiangli and Calvin,
>
> Thanks for the review. I've incorporated your comments in the following webrev:
>
> http://cr.openjdk.java.net/~iklam/jdk14/8227370_remove_shared_misc_info.v03/
> http://cr.openjdk.java.net/~iklam/jdk14/8227370_remove_shared_misc_info.v03-delta/
>
> Please take a look.
> Thanks
> - Ioi
>
>
> On 8/26/19 5:56 PM, Jiangli Zhou wrote:
>
> 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}
>>>
>>
>