RFR: 8222852: Reduce String concat combinator tree shapes by folding constants into prependers

[Claes Redestad]

On 2019-04-23 13:27, Aleksey Shipilev wrote:
> On 4/23/19 1:05 PM, Claes Redestad wrote:
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8222852
>> Webrev: http://cr.openjdk.java.net/~redestad/8222852/open.00/
> 
> Cool. Do we see the real-world examples where this helps? E.g. larger applications that have lots of
> concats? Asking because most apps do not have lots of concat shapes to begin with.

I can see some real but modest gains at use sites in the JDK code, e.g.,
java.util.logging bootstraps a few variants, and you get small
improvements already on bootstrapping obj + "foo" + obj at one site and
"foo" + obj + obj at another (-8 classes).

Most "real-world" benchmarks we run have not been built with a release
target of 9 or above, and even if they had most libraries they use are
likely to stay 8 compatible for some time. So it's anyone's guess what
gains we'd be able to expect on modern real-world applications.

> 
> I'd keep the switch in the second loop, like this:
> 
>    for (RecipeElement el : recipe.getElements()) {
>        switch (el.getTag()) {
>            case TAG_ARG:
>                ...
>                break;
>            case TAG_CONST:
>                // Constants are already handled in the code above.
>                break;
>            default:
>                throw new StringConcatException("Unhandled tag: " + el.getTag());
>        }
>    }

I figured that since the first switch would already check for unhandled
tags then a single test for TAG_ARG would suffice in the second, but I
have no strong opinion. Just thought it simplified the code flow a bit.

Thanks!

/Claes