RFR: 8263718: unused-result warning happens at os_linux.cpp

[David Holmes]

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> 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), 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.

>> 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), and it fixed unused-return as following:
> 
> 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
>