[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: Patch to inline os::SpinPause() for X86 on non-Windows OS
From: Karen Kinnear <karen.kinnear () oracle ! com>
Date: 2018-07-26 0:40:04
Message-ID: 85A4AE3E-62C9-4663-BC3E-BC2C64037FAB () oracle ! com
[Download RAW message or body]
Man.
> On Jul 25, 2018, at 8:08 PM, Man Cao <manc@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@oracle.com \
> <mailto:karen.kinnear@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@google.com <mailto:manc@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,@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,@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,@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,@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>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic