<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi all,<br>
<br>
I got some comments from Thomas Schatzl, mostly about comment and
assert changes. He also suggested to investigate combining current 2
steps of handling retaining survivor region into 1 step, so I filed
JDK-8223106.<br>
<br>
Webrev: <br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~sangheki/8220089/webrev.2/">http://cr.openjdk.java.net/~sangheki/8220089/webrev.2/</a><br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~sangheki/8220089/webrev.2_inc/">http://cr.openjdk.java.net/~sangheki/8220089/webrev.2_inc/</a><br>
Testing: hs-tier1<br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<div class="moz-cite-prefix">On 4/9/19 9:48 PM,
<a class="moz-txt-link-abbreviated" href="mailto:sangheon.kim@oracle.com">sangheon.kim@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:174f169f-4ccb-7567-7707-d6a7f7024aa8@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
Hi all,<br>
<br>
Please resume the review.<br>
The review for the blocking issue, JDK-8218049 is finished/pushed.<br>
<br>
Previously I requested to hold the review because of JDK-8218049
which addresses the problem of inconsistent data between
MemoryUsage.used() and actual used bytes. This CR - 8220089 would
change the number of survivor and eden regions which is related to
JDK-8218049.<br>
<br>
webrev.1 includes:<br>
1) JDK-8218049 related changes.<br>
- Changed to minimize direct use of G1EdenRegions and
G1SurvivorRegions. Instead changed to use
G1CollectedHeap::eden_regions_count() and
G1CollectedHeap::survivor_regions_count() which reflect the
retained region. G1EdenRegions::length() and
G1SurvivorRegions::length() returns only current region count.<br>
2) Minor refactoring such as renaming methods and updating
comments.<br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esangheki/8220089/webrev.1"
moz-do-not-send="true">http://cr.openjdk.java.net/~sangheki/8220089/webrev.1</a><br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esangheki/8220089/webrev.1_inc"
moz-do-not-send="true">http://cr.openjdk.java.net/~sangheki/8220089/webrev.1_inc</a><br>
<br>
Testing: hs-tier1 ~ 5, some benchmark tests(specjbb2015, 2005 and
specjvm2008) shows no significant difference.<br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<div class="moz-cite-prefix">On 3/28/19 12:51 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:sangheon.kim@oracle.com" moz-do-not-send="true">sangheon.kim@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:45151a57-4689-21e6-4bfb-a884d6b6c14d@oracle.com">Hi
all, <br>
<br>
Please hold reviewing this. <br>
<br>
During offline discussion with Thomas, he found a problem with
MemoryMXBeans. As proposed patch is changing survivor region to
mutator region, used bytes as survivor region will not count the
retained survivor region. This problem will be addressed by
JDK-8218049 but as this patch also would be affected, I propose
to hold reviewing this. <br>
<br>
Thanks, <br>
Sangheon <br>
<br>
<br>
On 3/27/19 10:49 PM, <a class="moz-txt-link-abbreviated"
href="mailto:sangheon.kim@oracle.com" moz-do-not-send="true">sangheon.kim@oracle.com</a>
wrote: <br>
<blockquote type="cite">Hi all, <br>
<br>
Can I have some reviews for the patch that changes to reuse
survivor region as mutator region? <br>
<br>
Currently survivor region is released at the end of gc and
then added to the next collection set. This means there would
be waste if the survivor region has enough free space. As the
survivor regions are converted to eden at the start of gc,
reusing the *last* survivor region as a mutator region would
maximize the use of heap region. This patch suggests to reuse
the survivor region as described if the remaining free space
satisfies a minimum threshold(currently, larger than or equal
to minimum TLAB). <br>
<br>
Some benchmarks(specjbb2015, specjbb2005, specjvm2008) that I
ran didn't have noticeable improvement. However, I think this
change would benefit more with NUMA implementation because the
NUMA implementation manages survivor region per NUMA node.
i.e. more wastes on multiple node system. <br>
<br>
CR: <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8220089"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8220089</a>
<br>
webrev: <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esangheki/8220089/webrev.0"
moz-do-not-send="true">http://cr.openjdk.java.net/~sangheki/8220089/webrev.0</a>
<br>
(based on JDK-8218668 webrev.1 which is under
review now) <br>
Testing: hs-tier1 ~ 5 <br>
<br>
Thanks, <br>
Sangheon <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>