RFR: 8245969: Simplify String concat constant folding

[Remi Forax]

Hi Claes,
so instead of having a prefix and a suffix, there is only a prefix, a suffix being seen as a prefix for the next iteration
and if at the end instead of just allocating an array, you allocate an array and stuff it with the last prefix.

Are you sure having a suffix at the end is uncommon enough so you use @Stable instead of final for NEW_ARRAY_SUFFIX ?

The name of the MH, NEW_ARRAY_SUFFIX, and the name of the method are different "newArrayPrepend", they should be identical, and i think the method should be called "newArrayWithSuffix".

I think i would prefer to have only one call to MethodHandles.foldArgumentsWithCombiner, with a variable combiner being either newArray() or newArrayWithSuffix(constant).

And
  constant = constant == null ? constantValue : constant + constantValue;
should be
  constant = constant == null ? constantValue : constant.concat(constantValue);
to avoid to create an intermediary StringBuilder (or worst if we are able to solve the boostraping issue of indy and try to use the StringConcatFactory inside itself).

I wonder if the code inside newArrayPrepend() can not be simplified given that we know that the suffix will be at the end of the array ?

regards,
Rémi

----- Mail original -----
> De: "Claes Redestad" <claes.redestad at oracle.com>
> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mercredi 27 Mai 2020 16:25:33
> Objet: RFR: 8245969: Simplify String concat constant folding

> Hi,
> 
> please review this patch to StringConcatFactory which (I think)
> simplifies the folding of constants around arguments by folding any
> suffix constant into the newArray combinator instead.
> 
> This simplifies the code in all prependers and in the general flow of
> the bootstrap code, at the cost of some added complexity around newArray
> and the new newArrayPrepend combinator. Slightly improves bootstrap and
> footprint overheads by not having to bind as many arguments to the
> prependers.
> 
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8245969
> Webrev: http://cr.openjdk.java.net/~redestad/8245969/open.00
> 
> Testing: tier1+2
> 
> Thanks!
> 
> /Claes