RFR(S) 8236236 Eliminate CDS md region and consolidate c++ vtable patching code

[Ioi Lam]

Hi Calvin, thanks for the review!

- Ioi

On 1/21/20 4:35 PM, Calvin Cheung wrote:
> Hi Ioi,
>
> The updated webrev looks good.
>
> thanks,
>
> Calvin
>
> On 1/15/20 5:53 PM, Ioi Lam wrote:
>> Hi Calvin,
>>
>> Thanks for the review. I have integrated the changes you suggested 
>> below.
>>
>> http://cr.openjdk.java.net/~iklam/jdk15/8236236_remove_cds_md_section.v02/ 
>>
>> http://cr.openjdk.java.net/~iklam/jdk15/8236236_remove_cds_md_section.v02.delta/ 
>>
>>
>> Changes since the last webrev:
>>
>> [1] The two test changes are from you.
>>
>> [2] FileMapInfo.java - I had to change SA to match the new location 
>> of the
>>     vtable, which is moved to the middle of the MC section. The new
>>     calculation in C code is
>>
>>        header->_mapped_base_address + header->_cloned_vtable_offset
>>
>>     The old SA code uses the words "address", "offset" and "value" very
>>     liberally. It's very hard to read, especially when I am now 
>> operating on
>>     fields named "address" and "offset". With the old code I would 
>> end up
>>     with variables called ...AddressAddress and ...OffsetFieldOffset,
>>     and my head would explode
>>
>>     So I decided to rewrite the whole FileMapInfo::initialize() 
>> method to
>>     make it a little easier to understand.
>>
>> [3] I removed "_start" from the names of a few pointers in filemap.hpp
>>     for brevity. E.g.,
>>
>>          serialized_data_start -> serialized_data
>>
>>     We have a char* here, so it's pretty obvious that it points to 
>> the "start"
>>     of the serialized data.
>>
>> Thanks
>> - Ioi
>>
>> On 1/2/2020 8:55 AM, Calvin Cheung wrote:
>>> Hi Ioi,
>>>
>>> The code changes look good. I saw the following tests still have 
>>> references to the MD region.
>>>
>>> appcds/SharedArchiveConsistency.java and 
>>> appcds/dynamicArchive/ArchiveConsistency.java
>>>
>>> Suggested change:
>>>
>>> bash-4.2$ hg diff 
>>> test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.javadiff 
>>> --git 
>>> a/test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.java 
>>> b/test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.java
>>> --- 
>>> a/test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.java
>>> +++ 
>>> b/test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.java
>>> @@ -74,7 +74,7 @@
>>>          "mc",          // MiscCode
>>>          "rw",          // ReadWrite
>>>          "ro",          // ReadOnly
>>> -        "md",          // MiscData
>>> +        "bm",          // relocation bitmaps
>>>          "first_closed_archive",
>>>          "last_closed_archive",
>>>          "first_open_archive",
>>> bash-4.2$
>>> bash-4.2$ hg diff 
>>> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/ArchiveConsistency.java
>>> diff --git 
>>> a/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/ArchiveConsistency.java 
>>> b/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/ArchiveConsistency.java 
>>>
>>> --- 
>>> a/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/ArchiveConsistency.java
>>> +++ 
>>> b/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/ArchiveConsistency.java
>>> @@ -49,7 +49,7 @@
>>>  public class ArchiveConsistency extends DynamicArchiveTestBase {
>>>      public static WhiteBox wb;
>>>      public static int int_size;        // size of int
>>> -    public static String[] shared_region_name = {"MiscCode", 
>>> "ReadWrite", "ReadOnly", "MiscData"};
>>> +    public static String[] shared_region_name = {"MiscCode", 
>>> "ReadWrite", "ReadOnly", "BitMap"};
>>>      public static int num_regions = shared_region_name.length;
>>>
>>>      public static void main(String[] args) throws Exception {
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>> On 12/19/19 3:51 PM, Ioi Lam wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8236236
>>>> http://cr.openjdk.java.net/~iklam/jdk15/8236236_remove_cds_md_section.v01/ 
>>>>
>>>>
>>>> This is the first step of:
>>>>
>>>> JDK-8234693 - Consolidate CDS static and dynamic archive dumping code
>>>>
>>>> My plan is to first make the two archives structurally alike, and then
>>>> merge the code that generates them.
>>>>
>>>> ---
>>>>
>>>> Currently the static archive has 4 regions, in the order of 
>>>> MC,RW,RO,MD,
>>>> but the dynamic archive has 3 regions, in the order of RW,RO,MC
>>>>
>>>> The difference in the number and ordering causes special handling 
>>>> code, e.g.,
>>>>
>>>> http://hg.openjdk.java.net/jdk/jdk/file/f33197adda9a/src/hotspot/share/memory/metaspaceShared.cpp#l2303 
>>>>
>>>>
>>>> The MD region is used only for the cloned CPP vtables. It's safe to
>>>> move these into the MC region.
>>>>
>>>> Also, for c++ vtable patching:
>>>>
>>>>   static  archive uses MetaspaceShared::patch_cpp_vtable_pointers()
>>>>   dynamic archive uses 
>>>> MetaspaceShared::fix_cpp_vtable_for_dynamic_archive().
>>>>
>>>> We should consolidate the code to just use the latter.
>>>>
>>>> Thanks
>>>> - Ioi
>>