Patch to inline os::SpinPause() for X86 on non-Windows OS

[Karen Kinnear]

Man,

Thank you for your proposal. The runtime is the correct team.

Could you please file an rfe under hotspot/runtime with the information below and the patch as well as any
tests you have run and any performance results you have?

That will help us track this information and find you a sponsor.

thanks,
Karen

> On Jul 18, 2018, at 9:53 PM, Man Cao <manc at google.com> wrote:
> 
> Hello,
> 
> 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?
> 
> 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.
> I found an interesting discussion about PAUSE and a microbenchmark in:
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2012-August/004352.html <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2012-August/004352.html>
> 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.
> 
> The patch is inlined and attached below:
> 
> diff --git a/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s b/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s
> --- a/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s
> +++ b/src/hotspot/os_cpu/bsd_x86/bsd_x86_32.s
> @@ -63,15 +63,6 @@
>          popl     %eax
>          ret
>  
> -        .globl  SYMBOL(SpinPause)
> -        ELF_TYPE(SpinPause, at function)
> -        .p2align 4,,15
> -SYMBOL(SpinPause):
> -        rep
> -        nop
> -        movl    $1, %eax
> -        ret
> -
>          # Support for void Copy::conjoint_bytes(void* from,
>          #                                       void* to,
>          #                                       size_t count)
> diff --git a/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s b/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s
> --- a/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s
> +++ b/src/hotspot/os_cpu/bsd_x86/bsd_x86_64.s
> @@ -46,15 +46,6 @@
>  
>  	.text
>  
> -        .globl SYMBOL(SpinPause)
> -        .p2align 4,,15
> -        ELF_TYPE(SpinPause, at function)
> -SYMBOL(SpinPause):
> -        rep
> -        nop
> -        movq   $1, %rax
> -        ret
> -
>          # Support for void Copy::arrayof_conjoint_bytes(void* from,
>          #                                               void* to,
>          #                                               size_t count)
> diff --git a/src/hotspot/os_cpu/linux_x86/linux_x86_32.s b/src/hotspot/os_cpu/linux_x86/linux_x86_32.s
> --- a/src/hotspot/os_cpu/linux_x86/linux_x86_32.s
> +++ b/src/hotspot/os_cpu/linux_x86/linux_x86_32.s
> @@ -42,15 +42,6 @@
>  
>  	.text
>  
> -        .globl  SpinPause
> -	.type   SpinPause, at function
> -        .p2align 4,,15
> -SpinPause:
> -        rep
> -        nop
> -        movl    $1, %eax
> -        ret
> -
>          # Support for void Copy::conjoint_bytes(void* from,
>          #                                       void* to,
>          #                                       size_t count)
> diff --git a/src/hotspot/os_cpu/linux_x86/linux_x86_64.s b/src/hotspot/os_cpu/linux_x86/linux_x86_64.s
> --- a/src/hotspot/os_cpu/linux_x86/linux_x86_64.s
> +++ b/src/hotspot/os_cpu/linux_x86/linux_x86_64.s
> @@ -38,15 +38,6 @@
>  
>  	.text
>  
> -        .globl SpinPause
> -        .align 16
> -        .type  SpinPause, at function
> -SpinPause:
> -        rep
> -        nop
> -        movq   $1, %rax
> -        ret
> -
>          # Support for void Copy::arrayof_conjoint_bytes(void* from,
>          #                                               void* to,
>          #                                               size_t count)
> diff --git a/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s b/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s
> --- a/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s
> +++ b/src/hotspot/os_cpu/solaris_x86/solaris_x86_64.s
> @@ -51,15 +51,6 @@
>          movq %fs:0x0,%rax
>          ret
>  
> -        .globl  SpinPause
> -        .align  16
> -SpinPause:
> -        rep
> -        nop
> -        movq    $1, %rax
> -        ret
> -
> -
>          / Support for void Copy::arrayof_conjoint_bytes(void* from,
>          /                                               void* to,
>          /                                               size_t count)
> diff --git a/src/hotspot/share/runtime/os.hpp b/src/hotspot/share/runtime/os.hpp
> --- a/src/hotspot/share/runtime/os.hpp
> +++ b/src/hotspot/share/runtime/os.hpp
> @@ -1031,6 +1031,13 @@
>  // of the global SpinPause() with C linkage.
>  // It'd also be eligible for inlining on many platforms.
>  
> +#if defined(X86) && !defined(_WINDOWS)
> +extern "C" int inline SpinPause() {
> +  __asm__ __volatile__ ("pause");
> +  return 1;
> +}
> +#else
>  extern "C" int SpinPause();
> +#endif
>  
>  #endif // SHARE_VM_RUNTIME_OS_HPP
> 
> -Man
> <inline_spinpause.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180725/ed703517/attachment.htm>