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

[Peter Levart]

Hi Claes,

On 4/28/19 8:24 PM, Peter Levart wrote:
> Hi Claes,
>
> If you had just one prepend method shape:
>
>       static long prepend(long indexCoder, byte[] buf, String prefix, 
> long value, String suffix) {
>           if (suffix != null) indexCoder = prepend(indexCoder, buf, 
> suffix);
>           indexCoder = prepend(indexCoder, buf, value);
>           if (prefix != null) indexCoder = prepend(indexCoder, buf, 
> prefix);
>           return indexCoder;
>       }
>
> ...then you could make the construction logic bind either null or 
> non-null constants for prefix/suffix and JIT would probably eliminate 
> dead-code paths in generated code. Meaning that specialization would 
> be performed by JIT instead of at link time. Perhaps this would have 
> the same max. performance with simplified wiring logic...
>
> What do you think?

This is what I meant by "simplified wiring logic":

http://cr.openjdk.java.net/~plevart/jdk-dev/8222852_StringConcatOpt/webrev.02p/

I haven't tried this though, but if it works correctly and if JIT 
inlines the strategy for each call site, it should also eliminate the 
dead code paths that are detected from null/non-null bound constants and 
the end result should be equivalent machine code. But that's just in 
theory...

Regards, Peter

>
> Regards, Peter
>
> On 4/26/19 6:13 PM, Claes Redestad wrote:
>> Not content with the fact that we generate one tree for "foo"+bar+baz
>> and another for foo+bar+"baz", I did a little "Hold my beer!"-style
>> experiment with a constant folder that greedily folds prefix and suffix
>> constants around an argument (so "foo"+bar would be prepended in one go,
>> and bar+"baz" in one go in the other tree, making the stem of both trees
>> shareable with no trailing constants having to be filtered in).
>>
>> And it works!
>>
>> Incremental: 
>> http://cr.openjdk.java.net/~redestad/8222852/open.02_hmb_inc/
>> Full: http://cr.openjdk.java.net/~redestad/8222852/open.02_hmb_full/
>>
>> It doubles the amount of statically defined prependers from 14 to 28,
>> but halves the theoretical maximum number of (larger and more expensive)
>> MH combinator trees.
>>
>> While I can see significant startup improvement on a range of tests
>> with this - and especially so for small, realistic ones - it's also a
>> bit of a mess. I will consider doing this as an immediate follow-up RFE,
>> if I can only clean it up to a point where the added bloat is
>> acceptable. One way to clean things up is to move from inner classes to
>> lambdas (like I've partially done already in this experiment). Turns out
>> to be effectively startup neutral here.
>>
>> /Claes
>>
>> On 2019-04-26 15:55, Claes Redestad wrote:
>>> Hi Peter,
>>>
>>> thanks for looking at this!
>>>
>>> On 2019-04-26 11:08, Peter Levart wrote:
>>>> Hi Claes,
>>>>
>>>> I wonder if it is even possible to create a test that would do 
>>>> something like the following:
>>>>
>>>>          String s = ...
>>>>          String s2 = s + "const1" + "const2" + s;
>>>>
>>>> ...since javac concatenates consecutive constants into a single 
>>>> constant. So this actually becomes:
>>>>
>>>>          String s2 = s + "const1const2" + s;
>>>>
>>>> ...in bytecode.
>>> <snip>
>>>> So what do you think? Is it important to test this or is it 
>>>> "obviously" correct?
>>>
>>> Adding an explicit sanity test for this seems reasonable to me, 
>>> since it's a case allowable by the StringConcatFactory that is not 
>>> expressible
>>> via javac:
>>>
>>> http://cr.openjdk.java.net/~redestad/8222852/open.02/
>>>
>>> Also cleaned up a few unused imports etc.
>>>
>>> Thanks!
>>>
>>> /Claes
>