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

[Calvin Cheung]

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
>