<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi Tom,
<br>
<br>
(Correct email thread this time, I hope.)<br>
<br>
Thanks for providing the great summary that eased the review
process. This change looks good to me.
<br>
<br>
Some minor comments:
<br>
<br>
g1Allocator.cpp
<br>
<br>
214 assert(_bottom >= _allocation_region->bottom(),
"inconsistent allocation state");
<br>
215 assert(_max <= _allocation_region->end(),
"inconsistent allocation state");
<br>
216 assert(_bottom <= old_top && old_top <= _max,
"inconsistent allocation state");
<br>
<br>
Maybe add err_msg() to see what the values actually were?
<br>
<br>
<br>
g1CollectedHeap.cpp
<br>
<br>
965 bool
<br>
966 G1CollectedHeap::check_archive_addresses()
<br>
<br>
We usually don’t have a linefeed after the return type. So, this
should be:
<br>
<br>
965 bool G1CollectedHeap::check_archive_addresses()
<br>
<br>
Same for G1CollectedHeap::alloc_archive_regions() and
G1CollectedHeap::fill_archive_regions()
<br>
<br>
<br>
g1MarkSweep.cpp
<br>
<br>
306 void G1MarkSweep::enable_archive_object_check() {
<br>
307 if (!_archive_check_enabled) {
<br>
<br>
Shouldn’t this be called exactly once, either from
G1RecordingAllocator::create_allocator() or
G1CollectedHeap::alloc_archive_regions()? In that case maybe the
“if” should be changed to an assert or guarantee that
!_archive_check_enabled holds.
<br>
<br>
Thanks,
<br>
Bengt
<br>
<br>
<br>
On 29/05/15 23:30, Tom Benson wrote:<br>
</div>
<blockquote cite="mid:5568DA67.5010808@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
Hi,<br>
Please review these changes for JDK-8042668, which constitute the
GC support for JDK-8059092 for storing interned strings in CDS
archives (JEP 250). The RFR for JDK-8059092 was recently posted
by Jiangli Zhou, and it would be best if overall comments could go
to that thread, with GC-specific comments here.<br>
<br>
JBS: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8042668">https://bugs.openjdk.java.net/browse/JDK-8042668</a><br>
Webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/8042668/webrev.00/">http://cr.openjdk.java.net/~brutisso/8042668/webrev.00/</a><br>
<br>
These changes add a new "archive" region type to G1. The
description field in JDK-8042668 contains an "Implementation
Notes" section which describes components of the design, and
should be useful for a code review. The overview:<br>
<blockquote>"Archive" regions are G1 regions that are not
modifiable by GC, being neither scavenged nor compacted, or even
marked in the object header. They can contain no pointers to
non-archive heap regions, and object headers point to shared CDS
metaspace (though this last point is not enforced by G1). Thus,
they allow the underlying hardware pages to be shared among
multiple JVM instances. <br>
<br>
In short, a dump-time run (using -Xshare:dump) will allocate
space in the Java heap for the strings which are to be shared,
copy the string objects and arrays to that space, and then
archive the entire address range in the CDS archive. At
restore-time (using -Xshare:on), that same heap range will be
allocated at JVM init time, and the archived data will be
mmap'ed into it. GC must treat the range as 'pinned,' never
moving or writing to any objects within it, so that cross-JVM
sharing will work. <br>
</blockquote>
Testing: All testing for JDK-8059092 included this code. Manual
testing with prototype calls to the new GC support was performed
before integration, along with JPRT and benchmark runs.<br>
<br>
Performance: The GC changes had no significant impact on SpecJBB,
JVM, or Dacapo benchmarks, run on x64 Linux. However, a small
(~1%) increase in Full GC times was seen in tests when the shared
string support was not in use, when runs are configured to
encounter them. When shared strings ARE in use, the impact could
be as high as 5% for a likely worst-case. Please see the JBS
entry for a discussion.<br>
<br>
Thanks, <br>
Tom<br>
<br>
<blockquote type="cite">
<div style="margin-top: 0px; margin-right: 0px; margin-bottom:
0px; margin-left: 0px;"><span style="font-family:'Helvetica';
color:rgba(0, 0, 0, 1.0);"><b>From: </b></span><span
style="font-family:'Helvetica';">Jiangli Zhou <<a
moz-do-not-send="true"
href="mailto:jiangli.zhou@oracle.com">jiangli.zhou@oracle.com</a>><br>
</span></div>
<div style="margin-top: 0px; margin-right: 0px; margin-bottom:
0px; margin-left: 0px;"><span style="font-family:'Helvetica';
color:rgba(0, 0, 0, 1.0);"><b>Subject: </b></span><span
style="font-family:'Helvetica';"><b>RFR JDK-8059092: Store
Interned Strings in CDS Archives</b><br>
</span></div>
<div style="margin-top: 0px; margin-right: 0px; margin-bottom:
0px; margin-left: 0px;"><span style="font-family:'Helvetica';
color:rgba(0, 0, 0, 1.0);"><b>Date: </b></span><span
style="font-family:'Helvetica';">May 29, 2015 at 12:39:27 PM
PDT<br>
</span></div>
<div style="margin-top: 0px; margin-right: 0px; margin-bottom:
0px; margin-left: 0px;"><span style="font-family:'Helvetica';
color:rgba(0, 0, 0, 1.0);"><b>To: </b></span><span
style="font-family:'Helvetica';">hotspot-runtime-dev <<a
moz-do-not-send="true"
href="mailto:hotspot-runtime-dev@openjdk.java.net">hotspot-runtime-dev@openjdk.java.net</a>><br>
</span></div>
<div style="margin-top: 0px; margin-right: 0px; margin-bottom:
0px; margin-left: 0px;"><span style="font-family:'Helvetica';
color:rgba(0, 0, 0, 1.0);"><b>Cc: </b></span><span
style="font-family:'Helvetica';"><a moz-do-not-send="true"
href="mailto:hotspot-gc-dev@openjdk.java.net">hotspot-gc-dev@openjdk.java.net</a><br>
</span></div>
<br>
<div>Greetings,<br>
<br>
Please review the changes for supporting interned String
objects and the underling character arrays in the CDS shared
archive. The webrevs listed below only include the runtime
changes. The webrev for GC specific changes will be sent out
by Tom Benson shortly.<br>
<br>
JEP<br>
<a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8059092">https://bugs.openjdk.java.net/browse/JDK-8059092</a><br>
<br>
Webrev<br>
hotspot :
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejiangli/8059092/webrev_hotspot.01/index.html">http://cr.openjdk.java.net/~jiangli/8059092/webrev_hotspot.01/index.html</a><br>
whitebox: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejiangli/8059092/webrev_wb.01/index.html">http://cr.openjdk.java.net/~jiangli/8059092/webrev_wb.01/index.html</a><br>
<br>
Summary<br>
The shared strings support is added on top of the basic CDS
function that enables the archiving of the interned String
objects and String’s underlying ‘value' array objects from the
java heap. During CDS dump time, all Strings referenced from
the string table is copied into a special region in java heap,
which is at the top of the dump time java heap. The special
region is referred as the string space. While copying the
String related objets, a compact table is created for storing
the references to the copied Strings. Both the compact table
and the string space are written into the CDS archive with the
rest of the class metadata during dumping. The compact table
for shared strings uses the same format as the shared symbol
table in CDS archive and shares implementations.<br>
<br>
At runtime, the string space is mapped at the same offset from
the narrow oop encoding base as dump time within the java
heap. That allows the shared strings to be ‘partially’
relocatable, which means the runtime java heap could be at
different address location and have different size from the
dump time java heap as long as the same narrow oop encoding
can be used. If the narrow oop encoding changes due to the
large difference between the dump-time and runtime heap sizes,
the shared string space from the CDS archive is ignored and
not mapped to the VM address space. <br>
<br>
The mapped string space is an ‘archive’ region in the java
heap. All shared objects residing within the region are not
collected or forwarded by GC. GC activities never write to the
memory pages that are mapped as the shared string space. The
identity hash of shared objects in the string space are
pre-computed during CDS archive dump time. The only possible
‘write’ to the shared string space at runtime is from
synchronization on the shared objects. That allows majority or
all mapped string memory to be sharable between different VM
processes. <br>
<br>
Only 64-bit process is supported for shared strings due to the
dependency on the narrow oop support. Windows is not supported
currently.<br>
<br>
Performance Results<br>
Memory<br>
Tested using about 3M of string data for memory measurement.
Memory results were measured using linux ps_mem tool.<br>
No Shared String<br>
Private + Shared = RAM used<span class="Apple-tab-span"
style="white-space:pre"> </span>Program Saving<br>
28.0 MiB + 110.5 KiB = 28.1 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java<br>
31.5 MiB + 12.6 MiB = 44.2 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (2)<br>
47.2 MiB + 12.7 MiB = 59.9 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (3)<br>
63.2 MiB + 12.8 MiB = 76.1 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (4)<br>
78.0 MiB + 12.9 MiB = 90.8 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (5)<br>
<br>
With Shared String<br>
27.6 MiB + 111.5 KiB = 27.7 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java 0.4M<br>
23.7 MiB + 16.3 MiB = 40.0 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (2) 4.2M<br>
35.3 MiB + 16.4 MiB = 51.7 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (3) 8.2M<br>
48.3 MiB + 16.5 MiB = 64.8 MiB<span class="Apple-tab-span"
style="white-space:pre"> </span>java (4) 11.3M<br>
60.6 MiB+ 16.5 MiB= 77.2MiB java(5) 13.6M<br>
<br>
Runtime Performance<br>
Tested on isolated linux-x64 machine.<br>
SpecJVM98<br>
==============================================================================<br>
logs.specjvm.before2:<br>
Benchmark Samples Mean Stdev
Geomean Weight<br>
specjvm98 10 603.39 23.25 <br>
==============================================================================<br>
logs.specjvm.after2:<br>
Benchmark Samples Mean Stdev %Diff
P Significant<br>
specjvm98 10 604.89 10.85 0.25
0.856 *<br>
==============================================================================<br>
<br>
No performance degradation shown in specjvm.<br>
<br>
Testing<br>
Tested with:<br>
Developed unit tests<br>
JPRT<br>
Full QA test cycle: vm.gc, vm.runtime, nsk.sysdict,
vm.metaspace, vm.quick, JCK: vm, lang, api, KS-24hrs, runThese<br>
Thanks,<br>
Jiangli<br>
<br>
</div>
</blockquote>
</blockquote>
<br>
</body>
</html>