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

[Man Cao]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190428/0c0ef492/attachment.htm>