Hi,
Quick observation before diving in further.
IIUC you are essentially adding a dummy index parameter to
StringConcatHelper.newString, which should always be zero, and that helps
compress LFs for the MH combinator:
341 static String newString(byte[] buf, int index, byte coder) {
342 // Use the private, non-copying constructor (unsafe!)
343 if (index != 0) {
344 throw new IllegalStateException("Storage is not completely
initialized, " + index + " bytes left");
345 }
346 return new String(buf, coder);
347 }
Did you consider replacing the if block with an assert? presumably if it is
non-zero it is an internal error?
Paul.
> On 8 Sep 2016, at 10:03, Claes Redestad <claes.redes...@oracle.com> wrote:
>
> Hi,
>
> StringConcatFactory$MethodHandleInlineCopyStrategy was made the default
> strategy in 9+120, which brought with it a number of startup
> regressions due to heavy use of MethodHandles when running the
> bootstrap method for each String concatenation. In exchange it allows
> for better peak performance, so it's been argued that we're striking a
> good trade-off already.
>
> However, after some analysis it appears we could collapse a few
> transformations where we were padding out combinators with dummy
> arguments as a mean to select an argument from the parameter list
> (which can also avoid the need to permute arguments in some cases). By
> introducing a method on j.l.invoke.MethodHandles to do this we can
> simplify the MethodHandleInlineCopyStrategy and produce on average 25%
> fewer LFs.
>
> By further folding the CHECK_INDEX into the newString method we avoid
> a few additional LF shapes from being created early on, while proving
> performance neutral on micros.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8165492
>
> Webrev: http://cr.openjdk.java.net/~redestad/8165492/webrev.01/
>
> Testing: RBT, no regressions observed on microbenchmarks[1], while
> recuperating ~25-30% of the largest startup regressions introduced in
> 9+120
>
> It's not the intent of this change to make this new foldArguments a
> part of the public API, since it's simply too late for 9. Instead we'd
> like to defer the discussion of possibly including this in the public
> API until a later time, which also gives us ample opportunity to
> examine other options - such as being better at not fully generating
> intermediary LFs.
>
> Thanks!
>
> /Claes
