<html><body><p><font face="Arial">Dear all,</font><br><br><font face="Arial">I update the webrev: </font><font face="Arial"><a href="http://cr.openjdk.java.net/~mhorie/8154736/webrev.09/">http://cr.openjdk.java.net/~mhorie/8154736/webrev.09/</a></font><br><br><font face="Arial">With the release barrier before the CAS, new_obj can be observed from other threads. If the CAS succeeds, the current thread can use new_obj without barriers. If the CAS fails, "o->forwardee()" is ordered with respect to CAS by accessing the same memory location "_mark", so no barriers needed. The order of (1) access to the forwardee and (2) access to forwardee's fields is preserved due to Release-Consume ordering on supported platforms. (The ordering between "new_obj = o->forwardee();" and logging or other usages is not changed.)</font><br><br><font face="Arial">Regarding the maintainability, the requirement is CAS(memory_order_release) as Release-Consume to be consistent with C++11. This requirement is necessary when a weaker platform like DEC Alpha is to be supported. On currently supported platforms, code change can be safe if the code meats this requirement (and the order of (1) access to the forwardee and (2) access to forwardee's fields is the natural way of coding).</font><br><br><br><font face="Arial">oop PSPromotionManager::copy_to_survivor_space(oop o) {</font><br><font face="Arial"> oop new_obj = NULL;</font><br><font face="Arial"> markOop test_mark = o->mark_raw();</font><br><br><font face="Arial"> if (!test_mark->is_marked()) { // The same test as "o->is_forwarded()"</font><br><font face="Arial"> :</font><br><font face="Arial"> Copy::aligned_disjoint_words((HeapWord*)o, (HeapWord*)new_obj, ...); </font><br><br><font face="Arial"> if (o->cas_forward_to(new_obj, test_mark)) { // CAS succeeds</font><br><font face="Arial"> :</font><br><font face="Arial"> new_obj->push_contents(this);</font><br><br><font face="Arial"> } else { // CAS fails</font><br><font face="Arial"> :</font><br><font face="Arial"> new_obj = o->forwardee();</font><br><font face="Arial"> }</font><br><font face="Arial"> } else {</font><br><font face="Arial"> :</font><br><font face="Arial"> new_obj = o->forwardee();</font><br><font face="Arial"> }</font><br><br><font face="Arial"> log_develop_trace(gc, scavenge)(..., new_obj->klass()->internal_name(), p2i((void *)o), p2i((void *)new_obj), new_obj->size());</font><br><font face="Arial"> return new_obj;</font><br><font face="Arial">}</font><br><font face="Arial"> </font><br><font face="Arial"> </font><br><font face="Arial">Best regards,</font><font face="Arial"><br></font><font face="Arial">--</font><font face="Arial"><br></font><font face="Arial">Michihiro,</font><font face="Arial"><br></font><font face="Arial">IBM Research - Tokyo</font><br><font face="Arial"> </font><br><font face="Arial"> </font><br><font face="Arial">----- Original message -----</font><font face="Arial"><br></font><font face="Arial">From: Kim Barrett <kim.barrett@oracle.com></font><font face="Arial"><br></font><font face="Arial">To: David Holmes <david.holmes@oracle.com></font><font face="Arial"><br></font><font face="Arial">Cc: Michihiro Horie <HORIE@jp.ibm.com>, ppc-aix-port-dev@openjdk.java.net, hotspot-dev@openjdk.java.net, hotspot-gc-dev@openjdk.java.net, Hiroshi H Horii <HORII@jp.ibm.com></font><font face="Arial"><br></font><font face="Arial">Subject: Re: RFR(M): 8154736: enhancement of cmpxchg and copy_to_survivor for ppc64</font><font face="Arial"><br></font><font face="Arial">Date: Fri, Apr 27, 2018 6:03 AM</font><font face="Arial"><br></font><font face="Arial"> </font><br><tt><font size="2">> On Apr 25, 2018, at 8:45 AM, David Holmes <david.holmes@oracle.com> wrote:</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> Hi Michihiro,</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> On 23/04/2018 8:33 PM, Michihiro Horie wrote:</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> Dear all,</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> I would like to ask reviews on 8154736 “enhancement of cmpxchg and</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> copy_to_survivor”. The change adds options to avoid expensive syncs with</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> compare-and-exchange. An experiment by using SPECjbb2015 showed 6%</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> improvement in critical-jOPS. This change focuses on ppc64 but would be</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> potentially beneficial for aarch64.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> Although discussions stopped at</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> </font></tt><a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-October/002718.html" target="_blank"><tt><u><font size="2" color="#0000FF">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-October/002718.html</font></u></tt></a><tt><font size="2"><br></font></tt><tt><font size="2">>> , I would like to restart the review by taking over Hiroshi's work if the</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">>> discussion is still open.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> So the very last comment there was about not implicitly assuming memory_order_consume, yet that has not been addressed in the proposal.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> Further the discussion on hotspot-runtime-dev through September and October was far more illuminating. I think my post here:</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> </font></tt><a href="http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-October/021617.html" target="_blank"><tt><u><font size="2" color="#0000FF">http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-October/021617.html</font></u></tt></a><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> and the closely following one from Thomas Schatzl summed up the concerns about the proposed changes.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> AFAICS the restarted proposal addresses none of those concerns but simply takes up where the previous implementation suggestion left off.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">> This is a proposal to change the memory ordering semantics of part of the shared GC code _not_ just the PPC64 implementation, but I have seen no analysis to demonstrate the correctness of such a proposal.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">I agree with David here. So far we've seen no such analysis. All we</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">have seen is a series of proposed changes and non-failing test</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">results, all of which have then been shown to have holes. (Among other</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">things, this suggests the set of tests being applied is inadequate.)</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">Part of the author's job is to convince reviewers that the proposed</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">change is correct. I'm not expecting a formal proof, but I am</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">expecting a lot more than has been provided so far.</font></tt><tt><font size="2"><br></font></tt><tt><font size="2"><br></font></tt><tt><font size="2">In this latest proposal, the conditional acquire doesn't look right to</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">me. If the logging is disabled so there is no acquire, the object is</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">then returned to the callers, who might do what with it? Is that safe?</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">For all callers? And is it stable through future maintenance? This is</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">not to say that I think making those acquires unconditional is</font></tt><tt><font size="2"><br></font></tt><tt><font size="2">sufficient.</font></tt><font face="Arial"><br></font><font face="Arial"> </font><br><font face="Arial"> </font><BR>
</body></html>