On 2020-05-29 19:32, fo...@univ-mlv.fr wrote:
----- Mail original -----
De: "Claes Redestad" <claes.redes...@oracle.com>
À: "Remi Forax" <fo...@univ-mlv.fr>
Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Vendredi 29 Mai 2020 18:47:03
Objet: Re: RFR: 8246152: Improve String concat bootstrapping
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/
I don't think you need to check MAX_INDY_CONCAT_ARG_SLOTS anymore, given that
now indy is based on invokeWithArguments instead of invoke.
Not sure what's changed, but let's leave this outside the scope of this
RFE?
And i think you should keep the null checks of constants upfront to not change
the semantics (you are allowing constant not referenced by a recipe to be null).
If anyone provides more constants than we use (including null values)
we'll fail the cCount != constants.length check later and throw a
StringConcatException anyway. So no real, semantic difference apart from
a change in what the exception message will say, which I think is OK,
no?
The catch should be:
catch (Error | RuntimeException e) {
if you want the StringConcatException to be propagated
StringConcatException is a regular Exception and is not thrown from
generateMHInlineCopy.
all the other changes looks good.
Thanks!
/Claes
Re-running tier1
/Claes
Rémi
