<html><body><p><font size="2">Hi Derek,</font><br><br><font size="2">Thank you very much for testing this change in AArch64 and giving observation on the result, which makes sense.</font><br><br><font size="2">I uploaded a new webrev based on the comments from Martin and Kim.</font><br><a href="http://cr.openjdk.java.net/~mhorie/8205908/webrev.02/"><font size="2">http://cr.openjdk.java.net/~mhorie/8205908/webrev.02/</font></a><br><br><br><font size="2">Best regards,</font><br><font size="2">--</font><br><font size="2">Michihiro,</font><br><font size="2">IBM Research - Tokyo</font><br><br><img width="16" height="16" src="cid:1__=8FBB0855DFB4F5838f9e8a93df938690918c8FB@" border="0" alt="Inactive hide details for "White, Derek" ---2018/07/10 05:48:39---Hi Michihiro, FYI, this patch does seem to help AArch64 also "><font size="2" color="#424282">"White, Derek" ---2018/07/10 05:48:39---Hi Michihiro, FYI, this patch does seem to help AArch64 also on SPECjbb to a lesser degree. This was</font><br><br><font size="2" color="#5F5F5F">From: </font><font size="2">"White, Derek" <Derek.White@cavium.com></font><br><font size="2" color="#5F5F5F">To: </font><font size="2">Michihiro Horie <HORIE@jp.ibm.com>, Kim Barrett <kim.barrett@oracle.com></font><br><font size="2" color="#5F5F5F">Cc: </font><font size="2">"hotspot-gc-dev@openjdk.java.net" <hotspot-gc-dev@openjdk.java.net>, Gustavo Romero <gromero@linux.vnet.ibm.com></font><br><font size="2" color="#5F5F5F">Date: </font><font size="2">2018/07/10 05:48</font><br><font size="2" color="#5F5F5F">Subject: </font><font size="2">RE: 8205908: Unnecessarily strong memory barriers in ParNewGeneration::copy_to_survivor_space</font><br><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br><br><br>Hi Michihiro,<br> <br>FYI, this patch does seem to help AArch64 also on SPECjbb to a lesser degree. This was benchmarked with very large young gen, so GC overhead is kept lower than you’d see in typical applications.<br>
<ul><li type="disc">Derek</ul> <br><b>From:</b> hotspot-gc-dev [<a href="mailto:hotspot-gc-dev-bounces@openjdk.java.net">mailto:hotspot-gc-dev-bounces@openjdk.java.net</a>] <b>On Behalf Of </b>Michihiro Horie<b><br>Sent:</b> Wednesday, July 04, 2018 4:26 AM<b><br>To:</b> Kim Barrett <kim.barrett@oracle.com><b><br>Cc:</b> hotspot-gc-dev@openjdk.java.net; Gustavo Romero <gromero@linux.vnet.ibm.com><b><br>Subject:</b> Re: 8205908: Unnecessarily strong memory barriers in ParNewGeneration::copy_to_survivor_space<br>
<p><font color="#FF0000">External Email</font><p><font size="2" color="#2F2F2F" face="Default San Serif">Hi Martin, Kim,</font><br><font size="2" color="#2F2F2F" face="Default San Serif"><br>Thank you for both of your comments.</font><br><font size="2" color="#2F2F2F" face="Default San Serif"><br>I missed the point that oopDesc::forward_to is invoked from several callers. Using OrderAccess:storestore() before the invocation of forward_to() would be a great idea, thanks.</font><br><font size="2" color="#2F2F2F" face="Default San Serif"><br>>I haven't looked carefully at the change, though I did find one part</font><font size="2" color="#2F2F2F">
</font><font size="2" color="#2F2F2F" face="Default San Serif"><br>>that I don't like. The new test of "order" in forward_to_atomic not</font><font size="2" color="#2F2F2F">
</font><font size="2" color="#2F2F2F" face="Default San Serif"><br>>only affects CMS, but also (uselessly) affects G1.<br>Please let me confirm your point. You mean I should give memory_order_acq_rel to forward_to_atomic, which uses tests as follows to hold the consistent meaning of acquire/release in forward_to_atomic? I agree it is not clear the test with release returns the forwardee with acquire.</font><br><font size="2" color="#2F2F2F" face="Default San Serif"><br>oop oopDesc::forward_to_atomic(oop p, atomic_memory_order order) {<br>:<br>while (!oldMark->is_marked()) {<br>if (order == memory_order_acq_rel) {<br>curMark = cas_set_mark_raw(forwardPtrMark, oldMark, memory_order_release);<br>} else {<br>curMark = cas_set_mark_raw(forwardPtrMark, oldMark, order);<br>}<br>}<br>:<br>}<br>if (order == memory_order_acq_rel) {<br>return forwardee_acquire();<br>}<br>return forwardee();<br>}</font><br><br><font size="2" face="Default San Serif"><br>Best regards,<br>--<br>Michihiro,<br>IBM Research - Tokyo</font><br><br><img src="cid:1__=8FBB0855DFB4F5838f9e8a93df938690918c8FB@" width="16" height="16" alt="Inactive hide details for Kim Barrett ---2018/07/04 05:41:02---> On Jul 3, 2018, at 4:25 AM, Michihiro Horie <HORIE@jp.ibm.com>"><font size="2" color="#424282">Kim Barrett ---2018/07/04 05:41:02---> On Jul 3, 2018, at 4:25 AM, Michihiro Horie <</font><a href="mailto:HORIE@jp.ibm.com"><u><font size="2" color="#0000FF">HORIE@jp.ibm.com</font></u></a><font size="2" color="#424282">> wrote: ></font><br><font size="2" color="#5F5F5F"><br>From: </font><font size="2">Kim Barrett <</font><a href="mailto:kim.barrett@oracle.com"><u><font size="2" color="#0000FF">kim.barrett@oracle.com</font></u></a><font size="2">></font><font size="2" color="#5F5F5F"><br>To: </font><font size="2">Michihiro Horie <</font><a href="mailto:HORIE@jp.ibm.com"><u><font size="2" color="#0000FF">HORIE@jp.ibm.com</font></u></a><font size="2">></font><font size="2" color="#5F5F5F"><br>Cc: </font><font size="2">"Doerr, Martin" <</font><a href="mailto:martin.doerr@sap.com"><u><font size="2" color="#0000FF">martin.doerr@sap.com</font></u></a><font size="2">>, "</font><a href="mailto:hotspot-gc-dev@openjdk.java.net"><u><font size="2" color="#0000FF">hotspot-gc-dev@openjdk.java.net</font></u></a><font size="2">" <</font><a href="mailto:hotspot-gc-dev@openjdk.java.net"><u><font size="2" color="#0000FF">hotspot-gc-dev@openjdk.java.net</font></u></a><font size="2">>, Gustavo Romero <</font><a href="mailto:gromero@linux.vnet.ibm.com"><u><font size="2" color="#0000FF">gromero@linux.vnet.ibm.com</font></u></a><font size="2">></font><font size="2" color="#5F5F5F"><br>Date: </font><font size="2">2018/07/04 05:41</font><font size="2" color="#5F5F5F"><br>Subject: </font><font size="2">Re: 8205908: Unnecessarily strong memory barriers in ParNewGeneration::copy_to_survivor_space</font><br><hr width="100%" size="2" align="left" noshade><br><br><br><font size="2" face="Courier New"><br>> On Jul 3, 2018, at 4:25 AM, Michihiro Horie <</font><a href="mailto:HORIE@jp.ibm.com"><u><font size="2" color="#0000FF" face="Courier New">HORIE@jp.ibm.com</font></u></a><font size="2" face="Courier New">> wrote:<br>> <br>> Hi Martin,<br>> <br>> Thanks a lot for your review. Sure, we need an OK from a CMS expert. Following is the new webrev:<br>> </font><a href="http://cr.openjdk.java.net/~mhorie/8205908/webrev.01/"><u><font size="2" color="#0000FF" face="Courier New">http://cr.openjdk.java.net/~mhorie/8205908/webrev.01/</font></u></a><font size="2" face="Courier New"><br>> <br>> >Seems like a user of the forwardee needs to rely on memory_order_consume in the current implementation. I guess it will be appreciated that you’re fixing this.<br>> Thank you for pointing out this issue in the original implementation. I newly inserted a release at "2.4. Set new_obj as forwardee [L1142]".<br>> <br>> Improvement of critical-jOPS in SPECjbb2015 was 10%, which is still a big number.<br>> <br>> <br>> Best regards,<br>> --<br>> Michihiro,<br>> IBM Research - Tokyo<br><br>CMS was deprecated in JDK 9, and has been on maintenance life-support<br>for some time. This complex-to-review performance enhancement was<br>proposed less than 48 hours before JDK 11 FC, and didn't receive any<br>reviews until after FC. Because of these factors, I don't think it<br>should be included in JDK 11. And if CMS gets removed in JDK 12 (I<br>don't know if that will happen), then this change would be rendered<br>entirely moot.<br><br>I haven't looked carefully at the change, though I did find one part<br>that I don't like. The new test of "order" in forward_to_atomic not<br>only affects CMS, but also (uselessly) affects G1.<br><br>I'm not going to be able to look at this carefully soon, as JDK 11 bug<br>fixing has a higher priority for me. Since I think CMS might soon not<br>be an issue, I'd really rather not look at it at all. I think this<br>change needs not just a CMS-expert reviewer, but someone who is<br>willing to maintain CMS (including any potential bug tail from this<br>change).<br><br><br></font><br><br><br><br><BR>
</body></html>