RFR (XS): 8220388: Increase -inlinehint-threshold for Clang to avoid G1 pause time regression

[Erik Joelsson]

The change looks ok from a build perspective.

Regarding optimization for size on Macos, I don't believe those settings 
have changed since Xcode changed from GCC to Clang. It could very well 
be that we would benefit from different optimization settings on Macos. 
Investigating that seems like out of scope for this change though, but 
certainly something that would be interesting if someone felt like doing 
an investigation.

/Erik

On 2019-04-28 22:30, Man Cao wrote:
> Hi Jiangli,
>
> Thanks for the feedback. I wasn't aware that we optimize for size on
> MacOSX, so I changed it to Linux-only:
> https://cr.openjdk.java.net/~manc/8220388/webrev.02/
>
> Size and performance comparison was included in
> https://bugs.openjdk.java.net/browse/JDK-8220388, copying sizes below:
> Sizes of libjvm.so:
> GoogGcc-default: 25369007
> GoogClang-default: 22946876
> GoogClang-100kInline: 24681265
>
> GCC version: 4.9
> Clang version: trunk r355087
>
> -Man
>
>
> On Sat, Apr 27, 2019 at 4:57 PM Jiangli Zhou <jianglizhou at google.com> wrote:
>
>> Hi Man,
>>
>> I have a question. Should the -inlinehint-threshold change be applied
>> to linux only? The following in flags-cflags.m4 indicates it's
>> optimized for size on MacOSX currently.
>>
>>    elif test "x$TOOLCHAIN_TYPE" = xclang; then
>>      if test "x$OPENJDK_TARGET_OS" = xmacosx; then
>>        # On MacOSX we optimize for size, something
>>        # we should do for all platforms?
>>        C_O_FLAG_HIGHEST_JVM="-Os"
>>        C_O_FLAG_HIGHEST="-Os"
>>        C_O_FLAG_HI="-Os"
>>        C_O_FLAG_NORM="-Os"
>>        C_O_FLAG_DEBUG_JVM=""
>>      else
>>        C_O_FLAG_HIGHEST_JVM="-O3"
>>        C_O_FLAG_HIGHEST="-O3"
>>        C_O_FLAG_HI="-O3"
>>        C_O_FLAG_NORM="-O2"
>>        C_O_FLAG_DEBUG_JVM="-O0"
>>      fi
>>
>> It would be helpful to share the binary size comparison with gcc and
>> clang (with -inlinehint-threshold=100000 change).
>>
>> Regards,
>> Jiangli
>>
>>
>>
>> On Fri, Apr 26, 2019 at 6:38 PM Man Cao <manc at google.com> wrote:
>>> (Adding build-dev at openjdk.java.net)
>>> Maybe some one from build-dev could review or comment on this change?
>>>
>>> -Man
>>>
>>> On Mon, Mar 11, 2019 at 12:55 PM Man Cao <manc at google.com> wrote:
>>>> Thanks for the suggestion.
>>>> Yes, I agree it makes sense to increase -inlinehint-threshold only for
>> "release" build.
>>>> However, I'm not sure if adding per-file CXX flags in
>> JvmOverrideFiles.gmk is a better approach.
>>>> The root problem is that Clang is more likely to ignore the "inline"
>> keyword than GCC, causing unexpected performance problems. G1 pause time
>> could be just one of many potential performance problems.
>>>> If we take the effort to identify which files need the
>> -inlinehint-threshold flag, we'd better take a step further to identify the
>> functions that should be ALWAYSINLINE.
>>>> Thus I think it is more maintainable to do one the following:
>>>> (1) Identify the functions that should be "ALWAYSINLINE" instead of
>> "inline", and avoid adding "-inlinehint-threshold" for Clang altogether.
>> This requires much more work.
>>>> (2) Increase "-inlinehint-threshold" for all files in "release" build
>> for Clang.
>>>> Note that -inlinehint-threshold is different from -inline-threshold, as
>> -inlinehint-threshold only affects methods marked as "inline" and shouldn't
>> unnecessarily bloat up the binary size for release build.
>>>> So I added an extra guard "x$DEBUG_LEVEL" = xrelease;" in
>> flags-cflags.m4:
>>>> https://cr.openjdk.java.net/~manc/8220388/webrev.01/
>>>>
>>>> -Man