RFR (S) 8227370 Remove SharedPathsMiscInfo

[Calvin Cheung]

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}
>>>>>
>