RFR: 8212995: Consider placing the Integer.IntegerCache and cached Integer objects in the closed archive heap region

[Calvin Cheung]

Hi Jiangli,

Code changes look good.
Just two minor nits in filemap.cpp:

1002   // First, map string regions as closed archive heap regions.
1003   // GC does not write into the regions.

The above comment was removed. I'd suggesting adding back another 
comment similar to the one at line 1012.
           // First, map closed archive heap regions, GC does not write 
into the regions.

1012     // Now, map open_archive heap regions, GC can write into the 
regions.

s/open_archive/open archive/

thanks,
Calvin

On 10/31/18, 12:45 PM, Jiangli Zhou wrote:
> On 10/31/18 12:08 PM, Jiangli Zhou wrote:
>
>> Hi Ioi,
>>
>> Here is an updated webrev with renaming of the 'is_shared' argument. 
>> I decided to go with your suggestion, 'is_closed_archive'.
>>
>> http://cr.openjdk.java.net/~jiangli/8212995/webrev.01/
>
> BTW, in above webrev, I also included a typo fix for the following 
> warning that Mandy found (thanks Mandy!)
>
> @@ -1324,11 +1329,11 @@
>    // header data
>    const char* prop = 
> Arguments::get_property("java.system.class.loader");
>    if (prop != NULL) {
>      warning("Archived non-system classes are disabled because the "
>              "java.system.class.loader property is specified (value = 
> \"%s\"). "
> -            "To use archived non-system classes, this property must 
> be not be set", prop);
> +            "To use archived non-system classes, this property must 
> not be set", prop);
>      _has_platform_or_app_classes = false;
>    }
>
> Thanks,
> Jiangli
>>
>> Thanks,
>>
>> Jiangli
>>
>>
>> On 10/30/18 4:19 PM, Jiangli Zhou wrote:
>>> Hi Ioi,
>>>
>>> On 10/30/18 3:00 PM, Ioi Lam wrote:
>>>
>>>> Hi Jiangli,
>>>>
>>>> This looks promising.
>>>>
>>>> Now a full review yet, but I am wondering about the name of the 
>>>> is_shared parameter
>>>>
>>>>   void add_subgraph_entry_field(int static_field_offset, oop v, 
>>>> bool is_shared);
>>>>
>>>> Since this is part of "heapShared", everything is "shared" in some 
>>>> sense of the word. It could be confusing to say something is more 
>>>> shared than other things which also shared ...
>>>>
>>>> How "is_closed_archive" instead?
>>> Yes, our 'shared' has broader meaning. "is_closed_archive" or 
>>> "is_closed_space" sounds good to me. I'll rename.
>>>
>>> Thanks,
>>> Jiangli
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>>
>>>> On 10/30/2018 01:57 PM, Jiangli Zhou wrote:
>>>>> Please review the following change for moving the archived 
>>>>> Integer.IntegerCache and it's cached Integer objects (256) to the 
>>>>> closed archiving heap region. The IntegerCache subgraph does not 
>>>>> contain any reference that's changed at runtime (good candidate 
>>>>> for sharing). Moving the whole subgraph into the closed archive 
>>>>> heap region allows the memory to be shared by different JVM 
>>>>> instances at runtime. The saving is 4K per JVM instance running 
>>>>> the same or different java application simultaneously. Although 4K 
>>>>> is not significant, in a larger picture the saving is much bigger 
>>>>> (4k * (JVM_instance_num - 1) * host_num).
>>>>>
>>>>> As part of the change, I also restructured the code to allow us to 
>>>>> plug in more shareable subgraphs in the closed archive heap region 
>>>>> for runtime footprint saving in the future.
>>>>>
>>>>> The 'st' space is renamed to 'ca' (closed archive) space since it 
>>>>> now contains other types of objects besides j.l.Strings.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~jiangli/8212995/webrev.00/
>>>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8212995
>>>>>
>>>>> Before:
>>>>>
>>>>> mc  space:      8416 [  0.0% of total] out of     12288 bytes [ 
>>>>> 68.5% used] at 0x0000000800000000
>>>>> rw  space:   3946640 [ 21.4% of total] out of   3948544 bytes 
>>>>> [100.0% used] at 0x0000000800003000
>>>>> ro  space:   7319328 [ 39.6% of total] out of   7319552 bytes 
>>>>> [100.0% used] at 0x00000008003c7000
>>>>> md  space:      2416 [  0.0% of total] out of      4096 bytes [ 
>>>>> 59.0% used] at 0x0000000800ac2000
>>>>> od  space:   6475944 [ 35.0% of total] out of   6479872 bytes [ 
>>>>> 99.9% used] at 0x0000000800ac3000
>>>>> st0 space:    438272 [  2.4% of total] out of    438272 bytes 
>>>>> [100.0% used] at 0x00000007ffc00000 <<<<<<<<<<
>>>>> oa0 space:    282624 [  1.5% of total] out of    282624 bytes 
>>>>> [100.0% used] at 0x00000007ff800000 <<<<<<<<<<
>>>>> total    :  18473640 [100.0% of total] out of  18485248 bytes [ 
>>>>> 99.9% used]
>>>>>
>>>>> After:
>>>>>
>>>>> mc  space:      8416 [  0.0% of total] out of     12288 bytes [ 
>>>>> 68.5% used] at 0x0000000800000000
>>>>> rw  space:   3946640 [ 21.4% of total] out of   3948544 bytes 
>>>>> [100.0% used] at 0x0000000800003000
>>>>> ro  space:   7319304 [ 39.6% of total] out of   7319552 bytes 
>>>>> [100.0% used] at 0x00000008003c7000
>>>>> md  space:      2416 [  0.0% of total] out of      4096 bytes [ 
>>>>> 59.0% used] at 0x0000000800ac2000
>>>>> od  space:   6475920 [ 35.0% of total] out of   6479872 bytes [ 
>>>>> 99.9% used] at 0x0000000800ac3000
>>>>> ca0 space:    442368 [  2.4% of total] out of    442368 bytes 
>>>>> [100.0% used] at 0x00000007ffc00000 <<<<<<<<<<
>>>>> oa0 space:    278528 [  1.5% of total] out of    278528 bytes 
>>>>> [100.0% used] at 0x00000007ff800000 <<<<<<<<<<
>>>>> total    :  18473592 [100.0% of total] out of  18485248 bytes [ 
>>>>> 99.9% used]
>>>>>
>>>>> Tested with appcds tests on linux-x64 locally. Running tier1-teir4 
>>>>> tests.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jiangli
>>>>>
>>>>>
>>>>
>>>
>>
>