RFR: 8165492: Reduce number of lambda forms generated by MethodHandleInlineCopyStrategy

[Paul Sandoz]

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.redestad at 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