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

[Ioi Lam]

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