RFR: 8245756: Reduce bootstrap cost of StringConcatFactory prependers

[Claes Redestad]

On 2020-05-26 20:02, Paul Sandoz wrote:
> I prefer the first revision from a simplicity perspective.
> 
> Are there any measurable benefits in the subsequent revision? It requires some careful reading, where the logic in the prepender is duplicated in the layering of the computeIfAbsent functions. Which, if needed, is fine.

Right, it depends on test. On the ObjStringCombos test there's no
real gain - on a case where there are more interleaving constants
it should have more of an effect.

I'll go ahead with the first version and we can think through if there
are ways to simplify this for a follow-up, maybe along with a fix to
remove prefix constants altogether. (the trick is to bind in the 
constants without binding more things directly into the main MH
combinator tree - the complex prependers reduced the possible tree
shapes a lot)

> 
> Are the existing test sufficient to cover these cases? You referenced ObjStringCombos and I wondering if that makes sense as a combo stress test?

Adding more stress tests that test all combinations at more arities
would be good. My bootstrap tests as written are a bit poor since I
don't do any verification apart from checking that they don't crash.
I'll file a standalone RFE to improve the coverage.

/Claes

> 
> Paul.
> 
>> On May 26, 2020, at 9:20 AM, Claes Redestad <claes.redestad at oracle.com> wrote:
>>
>> On 2020-05-26 00:48, forax at univ-mlv.fr wrote:
>>>> Not sure in which sense you mean inlining? Few of the methods in the
>>>> bootstrap code are likely hot enough to see inlining by a JIT - and the
>>>> resulting MHs should be identical, just not constructed over and over
>>>> again.
>>> I'm wondering if the code without the null tests was not simple enough to be inlined by c1,
>>> the MH creations tends to do a lot of checks that can be removed even by c1.
>>
>> Time spent in prepender() does not seem to be a significant contributor
>> to bootstrap overhead - time spent in MHs.insertArguments is, though.
>>
>> One thing that falls out naturally from this is that prefix can only be
>> non-null on the first prepender: "foo" + a + "bar" + b ... will bind
>> both "foo" and "bar" to the a-prepender, then b and subsequent
>> prependers will never see a prefix constant since it will be bound in
>> as a suffix of the preceding argument. Thus it makes sense to either
>> provide a caching mechanism for prependers whose prefix arguments are
>> pre-bound to null:
>>
>> http://cr.openjdk.java.net/~redestad/8245756/open.01
>>
>> We might be able to special-case the first prepender so that we can
>> remove the prefix argument from prependers in general, which might make
>> the final MH a bit smaller and easier for the JIT to optimize.
>>
>> Testing: tier1+2
>>
>> /Claes
>