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

Karen Kinnear karen.kinnear at oracle.com
Thu Jul 26 00:40:04 UTC 2018


Man.

> On Jul 25, 2018, at 8:08 PM, Man Cao <manc at google.com> wrote:
> 
> Thanks Karen for the response!
> I don't have a JBS account currently. I could ask my colleagues with JBS accounts to create an RFE for this issue, but probably I cannot directly post comments or performance results on JBS.
Sounds good - why don’t you work with a colleague who has a JBS account until you can become an Author.
> 
> I'm working on upstreaming more Google-local runtime and GC patches, so I can become an Author, according to:
> https://wiki.openjdk.java.net/display/general/JBS+Overview <https://wiki.openjdk.java.net/display/general/JBS+Overview>
> So far I have just contributed one patch: JDK-8193386.
> 
> Can I just post performance results on the mailing list and someone could copy the results when creating an RFE?
Sounds like you will be finding a colleague who can help to create the initial RFE. Feel free to work with
that colleague to add updates, or wait until you have the information to only need to ask them a favor once.

thanks,
Karen
> 
> -Man
> 
> 
> On Wed, Jul 25, 2018 at 2:56 PM Karen Kinnear <karen.kinnear at oracle.com <mailto:karen.kinnear at oracle.com>> wrote:
> 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 <mailto: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/ee362d1f/attachment.htm>


More information about the hotspot-gc-dev mailing list