RFR: 8243470: [macos] bring back O2 opt level for unsafe.cpp

[Erik Joelsson]

Looks good.

/Erik

On 2020-06-22 03:39, Vladimir Kempik wrote:
> Hello
> Thanks for looking into this Magnus.
>
> Here is updated version of webrev - http://cr.openjdk.java.net/~vkempik/8243470/webrev.01/ <http://cr.openjdk.java.net/~vkempik/8243470/webrev.01/>
> effectively it does exactly the same as previous version, I have removed the version check for clang.
>
>
> Thanks, Vladimir
>
>> 4 июня 2020 г., в 16:04, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> написал(а):
>>
>> On 2020-06-03 20:07, Magnus Ihse Bursie wrote:
>>> Thanks for cc:ing us.
>>>
>>> This is not the correct way to check for compiler versions. Nor is it the correct place.
>>>
>>> I don't have time for a full reply tonight, but will return with a better reply tomorrow.
>> Ok, I have now looked more into this.
>>
>> You are apparently want to check for Xcode version, not clang version. This is not the same. Note that clang can be used on Linux as well. Apple is basically dropping the normal clang versioning and replacing it with their own, messed-up version which somewhat resembles the corresponding Xcode version, but is not identical. Horray. :-/ (For those interested, have a look at https://gist.github.com/yamaya/2924292.)
>>
>> Anyway. We do not support Xcode prior to 8. You seem to have tested from 8 and up, so this patch should be applied unconditionally. That is, just remove the -O1 special thing.
>>
>> /Magnus
>>> /Magnus
>>>
>>> On 2020-06-03 19:47, Daniel D. Daugherty wrote:
>>>> Adding build-dev at ... since this is a build system change.
>>>>
>>>> As for the right HotSpot team, I'm not sure who should be making
>>>> the call on changing the way unsafe.cpp is built for macOSX. My guess
>>>> when I moved the bug to hotspot/runtime was that someone on the Runtime
>>>> team would know, but...
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>> On 6/3/20 1:38 PM, Vladimir Kempik wrote:
>>>>> Hello
>>>>>
>>>>> Can somebody please review this simple change ?
>>>>>
>>>>> Thanks
>>>>>
>>>>>> 6 мая 2020 г., в 12:43, Vladimir Kempik <vkempik at azul.com> написал(а):
>>>>>>
>>>>>> Adding hotspot-runtime-dev
>>>>>>
>>>>>>> 23 апр. 2020 г., в 18:26, Vladimir Kempik <vkempik at azul.com> написал(а):
>>>>>>>
>>>>>>>
>>>>>>> Hello
>>>>>>> Please review a fix for JDK-8243470
>>>>>>>
>>>>>>> Long time ago as part of JEP284: New HotSpot Build System this fix was applied to jdk9 https://bugs.openjdk.java.net/browse/JDK-8152666
>>>>>>> At that time it was decided to lower optimisation level for unsafe.cpp from O2 to O1 only for clang on Macosx.
>>>>>>>
>>>>>>> I suppose it was done due to issues in Set/Get<Object> helper functions where too optimistic optimisations were eliminating some null pointer checks. it was probably a clang bug.
>>>>>>> That issue could be checked with test jdk/test/sun/misc/CopyMemory.java.
>>>>>>>
>>>>>>> I believe that workaround (going from O2 to O1) produced this issue - https://bugs.openjdk.java.net/browse/JDK-8234963 (Thread.getStackTrace is slow with clang).
>>>>>>> JDK-8234963 can only be seen on mac with libjvm compiled by clang.
>>>>>>>
>>>>>>> Here I propose the patch which eliminates that workaround for clang 8+.
>>>>>>>
>>>>>>> I have tested clang versions 8/9/9.1/10, all of them showed good results:
>>>>>>> 1) CopyMemory test passes fine on 11/14/15.
>>>>>>> 2) jdk11/jdk14 passed tck. Regression testing were good as well. jdk15: no new failures in tck.
>>>>>>> 3) The testRandom "benchmark" from 8234963 shows great improvements on my machine, going down from ~200 seconds to ~150 seconds (the newer clang the better result). For comparision, gcc built libjvm for jdk11 shows ~130 seconds on my machine.
>>>>>>>
>>>>>>> The webrev: http://cr.openjdk.java.net/~vkempik/8243470/webrev.00/
>>>>>>>
>>>>>>> getStackTrace benchmark: http://cr.openjdk.java.net/~vkempik/8243470/TestRandom.java
>>>>>>>
>>>>>>> The bug: https://bugs.openjdk.java.net/browse/JDK-8243470
>>>>>>>
>>>>>>> Thanks, Vladimir