RFR: 8245756: Reduce bootstrap cost of StringConcatFactory prependers

[Claes Redestad]

On 2020-05-26 20:56, forax at univ-mlv.fr wrote:
> ----- 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.
> 

Ok, I'll go ahead and push that version.

/Claes

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