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

[Jiangli Zhou]

+1

Best,
Jiangli

On Mon, Apr 29, 2019 at 9:02 AM Erik Joelsson <erik.joelsson at oracle.com> wrote:
>
> 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