RFR: 8246152: Improve String concat bootstrapping

[Claes Redestad]

Hi Paul,

On 2020-05-29 21:57, Paul Sandoz wrote:
> This looks good.

Thanks for reviewing!

> 
>   204         String recipe = "\u0001".repeat(concatType.parameterCount());
> 
> IIUC, that’s a rather convoluted way of creating a List<String> from:
> 
>    Collections.nCopies(concatType.parameterCount(). (String) null);
> 
> Perhaps it makes sense to double down on the List representation with an internal method accepting that, which the public methods call?

My concern is that makeConcat is unlikely to be used in practice, so
micro-optimizing it at the expense of the default BSM seem like a bad
call. As discussed offline I'll add a short inline comment to this
effect:

         // This bootstrap method is unlikely to be used in practice,
         // avoid optimizing it at the expense of makeConcatWithConstants

Thanks!

/Claes

> 
> Paul.
> 
>> On May 29, 2020, at 9:47 AM, Claes Redestad <claes.redestad at oracle.com> wrote:
>>
>> Hi Rémi,
>>
>> thanks for looking at this and suggesting some improvements!
>>
>> On 2020-05-29 17:51, Remi Forax wrote:
>>> Hi Claes,
>>> For the code below the comment "Mock the recipe to reuse the concat generator code",
>>> i believe you can use String.repeat() that was introduced recently.
>>
>> Sure,
>>
>>> The code that parse the the receipe can be in its own method to make the code more readable,
>>> this method returns the list and use the StringBuilder internally.
>>
>> sure,
>>
>>> In generateMHInlineCopy,
>>>    element.get(0) and element.get(1) should be stored in local variables after "elements.size() == 2"
>>>    will make the code more readable
>>
>> sure,
>>
>>> In simpleConcat(),
>>>    should use a local variable 'mh', like the newString or newArrayWithSuffix
>>
>> yes!
>>
>> http://cr.openjdk.java.net/~redestad/8246152/open.01/
>>
>> Re-running tier1
>>
>> /Claes
>