RFR: 8245756: Reduce bootstrap cost of StringConcatFactory prependers

[forax at univ-mlv.fr]

----- Mail original -----
> De: "Claes Redestad" <claes.redestad at oracle.com>
> À: "Paul Sandoz" <paul.sandoz at oracle.com>
> Cc: "Remi Forax" <forax at univ-mlv.fr>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mardi 26 Mai 2020 20:31:51
> Objet: Re: RFR: 8245756: Reduce bootstrap cost of StringConcatFactory prependers

> 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)

I should have shut my big mouth,
i agree that the first patch is actually better.

> 
>> 
>> 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

Rémi

> 
>> 
>> 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