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.redes...@oracle.com>
> À: "core-libs-dev" <core-libs-dev@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

Reply via email to