RFR (S) 8227370 Remove SharedPathsMiscInfo

[Ioi Lam]

Hi Calvin,

Thanks for the review. I've created 
https://bugs.openjdk.java.net/browse/JDK-8230291
so we can mention this in the release notes.

Thanks
- Ioi

On 8/27/19 1:57 PM, Calvin Cheung wrote:
> Hi Ioi,
>
> The latest webrev looks good.
>
> In FileMapInfo::validate_non_existent_class_paths()
>
>  848     if (!ent->check_non_existent()) {
>  849       warning("Archived non-system classes are disabled because 
> the "
>  850               "file %s exists", ent->name());
>  851       _header->_has_platform_or_app_classes = false;
>  852     }
>
> Currently, with single archive case (no dynamic archive), we set the 
> UseSharedSpaces to false under the above condition.
>
> Should we document the new behavior?
>
> thanks,
>
> Calvin
>
> On 8/27/19 1:15 PM, Ioi Lam wrote:
>> 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}
>>>>>>
>>