RFR (S) 8227370 Remove SharedPathsMiscInfo

[Ioi Lam]

Hi Jiangli,

Thanks for the review. I will fix that and push.

- Ioi

On 8/27/19 12:51 PM, Jiangli Zhou wrote:
> 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}
>>>>