<html><body><p><font size="2">Hi Martin,</font><br><br><font size="2">Thank you for your comments. Yes, this change is significant on PPC64 as I showed a big improvement in SPECjbb2015 (27% better critical-jOPS).</font><br><br><font size="2">Changing the handle_evacuation_failure_par is not necessary. I could not observe the performance bottleneck in handle_evacuation_failure_par from the profiles,</font><br><br><font size="2">New webrev: </font><font size="2"><a href="http://cr.openjdk.java.net/~mhorie/8204524/webrev.01/">http://cr.openjdk.java.net/~mhorie/8204524/webrev.01/</a></font><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__=8FBB0839DFAE833C8f9e8a93df938690918c8FB@" border="0" alt="Inactive hide details for "Doerr, Martin" ---2018/06/12 18:47:59---Hi Michihiro, I have removed the mailing lists except hotspo"><font size="2" color="#424282">"Doerr, Martin" ---2018/06/12 18:47:59---Hi Michihiro, I have removed the mailing lists except hotspot-gc-dev which is the one for this revie</font><br><br><font size="2" color="#5F5F5F">From: </font><font size="2">"Doerr, Martin" <martin.doerr@sap.com></font><br><font size="2" color="#5F5F5F">To: </font><font size="2">Michihiro Horie <HORIE@jp.ibm.com>, "hotspot-gc-dev@openjdk.java.net" <hotspot-gc-dev@openjdk.java.net></font><br><font size="2" color="#5F5F5F">Cc: </font><font size="2">Kim Barrett <kim.barrett@oracle.com>, "david.holmes@oracle.com" <david.holmes@oracle.com>, Erik Osterlund <erik.osterlund@oracle.com></font><br><font size="2" color="#5F5F5F">Date: </font><font size="2">2018/06/12 18:47</font><br><font size="2" color="#5F5F5F">Subject: </font><font size="2">RE: RFR(M): 8204524: Unnecessary memory barriers in G1ParScanThreadState::copy_to_survivor_space</font><br><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br><br><br><font face="Calibri">Hi Michihiro,</font><br><font face="Calibri"> </font><br><font face="Calibri">I have removed the mailing lists except hotspot-gc-dev which is the one for this review.</font><br><font face="Calibri"> </font><br><font face="Calibri">Thank you for taking care of this PPC64 performance problem. I think we shouldn’t ship jdk11 on PPC64 without addressing it.</font><br><font face="Calibri"> </font><br><font face="Calibri">I guess handle_evacuation_failure_par is not performance critical, so I wonder if it needs to be part of this change. I haven’t checked if it’s correct.</font><br><font face="Calibri"> </font><br><font face="Calibri">Your description and change of copy_to_survivor_space fit to the comments and how the algorithm works. So it looks good to me.</font><br><font face="Calibri">I couldn’t find any requirement for memory barriers regarding the CAS. But we should have a G1 expert double-check that we haven’t missed anything.</font><br><font face="Calibri"> </font><br><font face="Calibri">Best regards,</font><br><font face="Calibri">Martin</font><br><font face="Calibri"> </font><br><font face="Calibri"> </font><br><b><font face="Calibri">From:</font></b><font face="Calibri"> Michihiro Horie [</font><font face="Calibri"><a href="mailto:HORIE@jp.ibm.com">mailto:HORIE@jp.ibm.com</a></font><font face="Calibri">] </font><b><font face="Calibri"><br>Sent:</font></b><font face="Calibri"> Donnerstag, 7. Juni 2018 08:01</font><b><font face="Calibri"><br>To:</font></b><font face="Calibri"> hotspot-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net; hotspot-gc-dev@openjdk.java.net</font><b><font face="Calibri"><br>Cc:</font></b><font face="Calibri"> Kim Barrett <kim.barrett@oracle.com>; Gustavo Bueno Romero <gromero@br.ibm.com>; david.holmes@oracle.com; Erik Osterlund <erik.osterlund@oracle.com>; Doerr, Martin <martin.doerr@sap.com></font><b><font face="Calibri"><br>Subject:</font></b><font face="Calibri"> RFR(M): 8204524: Unnecessary memory barriers in G1ParScanThreadState::copy_to_survivor_space</font><br><font face="Calibri"> </font><p><font size="2" color="#2F2F2F">Dear all,</font><br><font size="2" color="#2F2F2F"><br>Would you please review the following change?</font><br><font size="2" color="#2F2F2F"><br>Bug: </font><a href="https://bugs.openjdk.java.net/browse/JDK-8204524"><u><font size="2" color="#0000FF">https://bugs.openjdk.java.net/browse/JDK-8204524</font></u></a><font size="2" color="#2F2F2F"><br>Webrev: </font><a href="http://cr.openjdk.java.net/~mhorie/8204524/webrev.00"><u><font size="2" color="#0000FF">http://cr.openjdk.java.net/~mhorie/8204524/webrev.00</font></u></a><br><font size="2" color="#2F2F2F"><br>G1ParScanThreadState::copy_to_survivor_space tries to move live objects to a different location. It uses a forwarding technique and allows multiple threads to compete for performing the copy step.</font><font size="2" color="#2F2F2F" face="MS Gothic">
</font><br><font size="2" color="#2F2F2F"><br>A copy is performed after a thread succeeds in the CAS. CAS-failed threads are not allowed to dereference the forwardee concurrently. Current code is already written so that CAS-failed threads do not dereference the forwardee. Also, this constraint is documented in a caller function mark_forwarded_object as “the object might be in the process of being copied by another worker so we cannot trust that its to-space image is well-formed”.</font><ul><ul type="disc"><li><font size="2" color="#2F2F2F" face="Calibri">There is no copy that must finish before the CAS.</font><li><font size="2" color="#2F2F2F" face="Calibri">Threads that failed in the CAS must not dereference the forwardee.</font></ul></ul><font size="2" color="#2F2F2F" face="Calibri"><br>Therefore, no fence is necessary before and after the CAS.</font><font face="Calibri"><br></font><font size="2" color="#2F2F2F" face="Calibri"><br>I measured SPECjbb2015 with this change. As a result, critical-jOPS performance improved by 27% on POWER8.</font><font face="Calibri"><br><br></font><font size="2" face="Calibri"><br>Best regards,<br>--<br>Michihiro,<br>IBM Research - Tokyo</font><br><BR>
</body></html>