<div dir="ltr"><div>Thanks Karen for the response!</div><div>I don't have a JBS account currently. I could ask my colleagues with JBS accounts to create an RFE for this issue, but <span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">probably </span>I cannot directly post comments or performance results on JBS.</div><div><br></div><div>I'm working on upstreaming more Google-local runtime and GC patches, so I can become an Author, according to:</div><div><a href="https://wiki.openjdk.java.net/display/general/JBS+Overview">https://wiki.openjdk.java.net/display/general/JBS+Overview</a><br></div><div>So far I have just contributed one patch: JDK-8193386.<br></div><div><br></div><div>Can I just post performance results on the mailing list and someone could copy the results when creating an RFE?</div><div><br></div><div>-Man<br></div><br></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 25, 2018 at 2:56 PM Karen Kinnear <<a href="mailto:karen.kinnear@oracle.com" target="_blank">karen.kinnear@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Man,<div><br></div><div>Thank you for your proposal. The runtime is the correct team.</div><div><br></div><div>Could you please file an rfe under hotspot/runtime with the information below and the patch as well as any</div><div>tests you have run and any performance results you have?</div><div><br></div><div>That will help us track this information and find you a sponsor.</div><div><br></div><div>thanks,</div><div>Karen<br><div><br><blockquote type="cite"><div>On Jul 18, 2018, at 9:53 PM, Man Cao <<a href="mailto:manc@google.com" target="_blank">manc@google.com</a>> wrote:</div><br class="m_430936533030427191m_7612539909124851865m_-5151025259025481513Apple-interchange-newline"><div><div dir="ltr"><div>Hello,</div><div><br></div><div>The Java platform team at Google has maintained a local patch to inline os::SpinPause() since 2014. We would like to upstream this patch to OpenJDK. Could someone sponsor this patch?</div><div><br></div><div>It is difficult to demonstrate performance improvement in Java benchmarks. It is more of a code refactoring to better utilize modern GCC. It partly addresses the comment about inlining SpinPause() above its declaration in os.hpp.</div><div>I found an interesting discussion about PAUSE and a microbenchmark in:<br></div><div><a href="http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2012-August/004352.html" target="_blank">http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2012-August/004352.html</a><br></div><div>However, the microbenchmark has a large variance in our experiment, making it difficult to tell if there's any benefit from inlining PAUSE. Inlining PAUSE does seem to reduce the variance a bit.</div><div><br></div><div>The patch is inlined and attached below:</div><div><br></div><div><div>diff --git a/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s b/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s<br></div><div>--- a/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s</div><div>+++ b/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s</div><div>@@ -63,15 +63,6 @@</div><div>         popl     %eax</div><div>         ret</div><div> </div><div>-        .globl  SYMBOL(SpinPause)</div><div>-        ELF_TYPE(SpinPause,@function)</div><div>-        .p2align 4,,15</div><div>-SYMBOL(SpinPause):</div><div>-        rep</div><div>-        nop</div><div>-        movl    $1, %eax</div><div>-        ret</div><div>-</div><div>         # Support for void Copy::conjoint_bytes(void* from,</div><div>         #                                       void* to,</div><div>         #                                       size_t count)</div><div>diff --git a/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s b/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s</div><div>--- a/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s</div><div>+++ b/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s</div><div>@@ -46,15 +46,6 @@</div><div> </div><div> <span style="white-space:pre-wrap">  </span>.text</div><div> </div><div>-        .globl SYMBOL(SpinPause)</div><div>-        .p2align 4,,15</div><div>-        ELF_TYPE(SpinPause,@function)</div><div>-SYMBOL(SpinPause):</div><div>-        rep</div><div>-        nop</div><div>-        movq   $1, %rax</div><div>-        ret</div><div>-</div><div>         # Support for void Copy::arrayof_conjoint_bytes(void* from,</div><div>         #                                               void* to,</div><div>         #                                               size_t count)</div><div>diff --git a/src/hotspot/os_cpu/linux_x86/linux_x86_32.s b/src/hotspot/os_cpu/linux_x86/linux_x86_32.s</div><div>--- a/src/hotspot/os_cpu/linux_x86/linux_x86_32.s</div><div>+++ b/src/hotspot/os_cpu/linux_x86/linux_x86_32.s</div><div>@@ -42,15 +42,6 @@</div><div> </div><div> <span style="white-space:pre-wrap">       </span>.text</div><div> </div><div>-        .globl  SpinPause</div><div>-<span style="white-space:pre-wrap">    </span>.type   SpinPause,@function</div><div>-        .p2align 4,,15</div><div>-SpinPause:</div><div>-        rep</div><div>-        nop</div><div>-        movl    $1, %eax</div><div>-        ret</div><div>-</div><div>         # Support for void Copy::conjoint_bytes(void* from,</div><div>         #                                       void* to,</div><div>         #                                       size_t count)</div><div>diff --git a/src/hotspot/os_cpu/linux_x86/linux_x86_64.s b/src/hotspot/os_cpu/linux_x86/linux_x86_64.s</div><div>--- a/src/hotspot/os_cpu/linux_x86/linux_x86_64.s</div><div>+++ b/src/hotspot/os_cpu/linux_x86/linux_x86_64.s</div><div>@@ -38,15 +38,6 @@</div><div> </div><div> <span style="white-space:pre-wrap">    </span>.text</div><div> </div><div>-        .globl SpinPause</div><div>-        .align 16</div><div>-        .type  SpinPause,@function</div><div>-SpinPause:</div><div>-        rep</div><div>-        nop</div><div>-        movq   $1, %rax</div><div>-        ret</div><div>-</div><div>         # Support for void Copy::arrayof_conjoint_bytes(void* from,</div><div>         #                                               void* to,</div><div>         #                                               size_t count)</div><div>diff --git a/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s b/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s</div><div>--- a/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s</div><div>+++ b/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s</div><div>@@ -51,15 +51,6 @@</div><div>         movq %fs:0x0,%rax</div><div>         ret</div><div> </div><div>-        .globl  SpinPause</div><div>-        .align  16</div><div>-SpinPause:</div><div>-        rep</div><div>-        nop</div><div>-        movq    $1, %rax</div><div>-        ret</div><div>-</div><div>-</div><div>         / Support for void Copy::arrayof_conjoint_bytes(void* from,</div><div>         /                                               void* to,</div><div>         /                                               size_t count)</div><div>diff --git a/src/hotspot/share/runtime/os.hpp b/src/hotspot/share/runtime/os.hpp</div><div>--- a/src/hotspot/share/runtime/os.hpp</div><div>+++ b/src/hotspot/share/runtime/os.hpp</div><div>@@ -1031,6 +1031,13 @@</div><div> // of the global SpinPause() with C linkage.</div><div> // It'd also be eligible for inlining on many platforms.</div><div> </div><div>+#if defined(X86) && !defined(_WINDOWS)</div><div>+extern "C" int inline SpinPause() {</div><div>+  __asm__ __volatile__ ("pause");</div><div>+  return 1;</div><div>+}</div><div>+#else</div><div> extern "C" int SpinPause();</div><div>+#endif</div><div> </div><div> #endif // SHARE_VM_RUNTIME_OS_HPP</div></div><div><br></div><div>-Man<br></div></div>
<span id="m_430936533030427191m_7612539909124851865m_-5151025259025481513cid:f_jjrvy2sw0"><inline_spinpause.patch></span></div></blockquote></div><br></div></div></blockquote></div>