RFR: 8263718: unused-result warning happens at os_linux.cpp
David Holmes
david.holmes at oracle.com
Fri Mar 19 06:08:09 UTC 2021
On 19/03/2021 3:56 pm, Thomas Stüfe wrote:
>
>
> On Fri, Mar 19, 2021 at 6:49 AM David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
> Okay so this may all be moot :)
>
> On 19/03/2021 3:09 pm, David Holmes wrote:
> > On 19/03/2021 10:17 am, Yasumasa Suenaga wrote:
> >> On Fri, 19 Mar 2021 00:02:21 GMT, Yasumasa Suenaga
> >> <ysuenaga at openjdk.org <mailto:ysuenaga at openjdk.org>> wrote:
> >>
> >>>>> The alloca was added as a performance boost for hyperthreaded
> systems
> >>>>> back in 2003 for JDK 5:
> >>>>>
> >>>>> "A per-thread offset was added to each thread's stack to
> randomize the
> >>>>> cachelines of hot stack frames (aka, stack coloring)."
> >>>>
> >>>> IIUC it relates to share DTLB between two logical processors when
> >>>> Hyperthreading is enabled.
> >>>>
> >>>>> I'm running some of our benchmarks to see if removing it makes a
> >>>>> difference ... but chances are I'm not going to be running it
> on the
> >>>>> kind of machines for which it was introduced.
> >>>>
> >>>> Thanks!
> >>>>
> >>>>>> If we decide to remain this code, we need to avoid
> unused-result
> >>>>>> warning from GCC. I think we can use pragma because other
> pragmas
> >>>>>> (format-nonliteral, format-security, stringop-truncation") are
> >>>>>> used in HotSpot. In addition, this behavior does not seem to
> >>>>>> determine to be a bug in GCC (status is UNCONFIRMED). However I
> >>>>>> will agree to use `(void)!` if it is still preferred based
> on them.
> >>>>>
> >>>>> I'd reluctantly prefer to use the pragma as an official
> mechanism for
> >>>>> avoiding the warning.
> >>>>
> >>>> Ok, OpenJDK folks who discuss in this PR are not prefer to use
> >>>> pragma, so I will use `(void)!` if it is needed.
> >>>> Let's see the result of benchmark and discuss what should we do.
> >>>
> >>>>> The alloca was added as a performance boost for hyperthreaded
> systems
> >>>>> back in 2003 for JDK 5:
> >>>>> "A per-thread offset was added to each thread's stack to
> randomize the
> >>>>> cachelines of hot stack frames (aka, stack coloring)."
> >>>>
> >>>> IIUC it relates to share DTLB between two logical processors when
> >>>> Hyperthreading is enabled.
> >>>
> >>> `alloca()` call might be less effective if ASLR is enabled. It
> can be
> >>> configured by
> >>>
> [randomize_va_space](https://www.kernel.org/doc/html/latest/admin-guide/sysctl/kernel.html#randomize-va-space
> <https://www.kernel.org/doc/html/latest/admin-guide/sysctl/kernel.html#randomize-va-space>),
>
> >>> and I guess it is enabled in a lot of x86 systems.
> >
> > It relates to L1 cache index collisions primarily so only impacts
> > hyperthreading on Intel CPUs (it also impacted CMT on SPARC).
> I've been
> > told that ASLR may render the manual stack-coloring unnecessary
> (as well
> > as ineffective), but then we'd need to establish which platforms
> have
> > that enabled, and whether we can tell.
> >
> > The benchmarking has been inconclusive - no observable
> differences. But
> > the benchmarking machines (and indeed the benchmarks) may not be
> > representative of the context where this optimization is needed.
> >
> > It has also been raised (as it was here) whether the alloca is
> even left
> > in place by the compiler. Something I have yet to check.
>
> call _ZN6Thread26record_stack_base_and_sizeEv at PLT
> .LVL2492:
> .loc 3 666 3 is_stmt 1 view .LVU7882
> .loc 3 667 3 view .LVU7883
> .LBB6399:
> .LBI6399:
> .loc 3 1359 5 view .LVU7884
> .LBB6400:
> .loc 3 1360 3 view .LVU7885
> .loc 3 1360 18 is_stmt 0 view .LVU7886
> call getpid at PLT
> .LVL2493:
> .LBE6400:
> .LBE6399:
> .loc 3 668 3 is_stmt 1 view .LVU7887
> .loc 3 670 36 is_stmt 0 view .LVU7888
> movq %r13, %rdi
> .loc 3 668 3 view .LVU7889
> addl $1, _ZZL19thread_native_entryP6ThreadE7counter(%rip)
> .loc 3 670 3 is_stmt 1 view .LVU7890
> .loc 3 670 36 is_stmt 0 view .LVU7891
> call _ZN6Thread25initialize_thread_currentEv at PLT
>
> Appears the alloca has been elided, along with the arithmetic.
>
> Don't know if that is true for other platforms.
>
> David
> -----
>
>
> Which would explain why we don't see effects in benchmarks. Question is,
> do we repair this or remove it.
I'm trying a repair and re-running the benchmarks.
The repair is simply:
static void* _stack_pad = alloca(...);
David
-----
> ..Thomas
>
> >>> BTW this is not the first time we have had this issue with gcc
> making a
> >>> function deprecated and casting to void not fixing it.
> Unfortunately I
> >>> can't remember enough of the previous case's details to
> actually look it
> >>> up and see what we resolved to do. :(
> >>
> >> I found
> >> [JDK-6879689](https://bugs.openjdk.java.net/browse/JDK-6879689
> <https://bugs.openjdk.java.net/browse/JDK-6879689>), and
> >> it fixed unused-return as following:
> >>
> >>
> https://github.com/openjdk/jdk/blob/9b5a9b61899cf649104c0ff70e14549f64a89561/src/hotspot/share/adlc/archDesc.cpp#L1082-L1083
> <https://github.com/openjdk/jdk/blob/9b5a9b61899cf649104c0ff70e14549f64a89561/src/hotspot/share/adlc/archDesc.cpp#L1082-L1083>
>
> >>
> >>
> >> I prefer it rather than `(void)!`.
> >
> > But introducing a local (which was already suggested earlier)
> that is
> > unused in a product build may also trigger a warning from the
> compiler
> > and we are back where we started.
> >
> > David
> >
> >> -------------
> >>
> >> PR: https://git.openjdk.java.net/jdk/pull/3042
> <https://git.openjdk.java.net/jdk/pull/3042>
> >>
>
More information about the hotspot-dev
mailing list