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.bur...@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@... 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 <vkem...@azul.com> написал(а): >>>>> >>>>> Adding hotspot-runtime-dev >>>>> >>>>>> 23 апр. 2020 г., в 18:26, Vladimir Kempik <vkem...@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 >>> >>
