RFR 8208458: Simplify and inline os::SpinPause() for non-Windows OS on X86
Man Cao
manc at google.com
Fri Jul 27 17:27:46 UTC 2018
Hi all,
JC kindly created an RFE and webrev for me, could someone review and
sponsor this change?
http://cr.openjdk.java.net/~jcbeyler/8208458/webrev.00/
I also reran performance experiments on a machine with more cores and lower
noises. Results are attached in the RFE.
-Man
On Wed, Jul 25, 2018 at 5:40 PM Karen Kinnear <karen.kinnear at oracle.com>
wrote:
> 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
> 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>
> 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> 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
>> 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>
>>
>>
>
More information about the hotspot-runtime-dev
mailing list