[External] : Re: Getting back into indy...need a better argument collector!

[Jorn Vernee]

Okay, the diagram turned out more crude than intended due to automatic 
email formatting.

See here instead: http://cr.openjdk.java.net/~jvernee/Collector_Diag.txt

Jorn

On 05/04/2021 21:16, Jorn Vernee wrote:
> Maybe you could do a fold with  a 'filler' that fills multiple 
> elements at a time to reduce the depth.
>
> e.g. declare a bunch of:
>
> void fillArray(Object[] arr, int startPos, Object a0,  Object a1, ...) {
>     arr[startPos + 0] = a0;
>     arr[startPos + 1] = a1;
>     ...
> }
>
> With differing arities, and then use a series of collectArguments 
> calls to collect the arguments into the final array in chunks (of e.g. 
> size 10). Merge all the array parameters together using 
> permuteArguments, and then finally stick an array constructor in that 
> parameter slot using collectArguments to create the array.
>
> See here a crude diagram:
>
> result( Object,Object,Object,       Object,Object,Object)void
>                                                    |      | 
> |             |      |      |
>                  arity                             |      | 
> |             |      |      |
>                    |                               |      | 
> |             |      |      |
>                    V                               |      | 
> |             |      |      |
>   Object[]::<init>(int)Object[]                    | | |             
> |      |      |
>                         |            startPos=0    |      | 
> |             |      |      |
>         |----------|------------|        |         |      | 
> |             |      |      |
>         |          |            V        V         V      V 
> V             |      |      |
>         |          | fillArray(Object[],int, 
> Object,Object,Object)void     |      |      |
>         | |                                                   | |      
> |      |
>         |          | startPos=3                        | |      |      |
>         |          |------------| | |       |      |      |
>         |                       V V |       V      V      V
>         | fillArray(Object[],int,                           | 
> Object,Object,Object)void
> | |                           |
>         |       |---------- collectArguments 
> --------------------------<---------------------------<
>         V       V
> target(Object[]  )void
>
> Where Object[] and Object can be replaced with sharper types.
>
> Although that would require writing a bunch of hard-coded fillers, one 
> for each primitive type and then one for reference types X the chunk 
> size you want to support.
>
> HTH,
> Jorn
>
> On 02/04/2021 10:00, Charles Oliver Nutter wrote:
>> First attempt at a workaround seems to be a wash. I rolled back to my
>> older logic (that does not use a hand-crafted collector method) to
>> come up with a pure-MethodHandle workaround for asCollector. I came up
>> with this (using InvokeBinder):
>>
>> ```
>> MethodHandle constructArray = Binder.from(arrayType, Object[].class)
>>          .fold(MethodHandles.arrayLength(Object[].class))
>>          .dropLast()
>>          .newArray();
>>
>> MethodHandle transmuteArray = Binder.from(arrayType, Object[].class)
>>          .fold(constructArray)
>>          .appendInts(0, 0, count)
>>          .permute(1, 2, 0, 3, 4)
>>          .cast(ARRAYCOPY.type().changeReturnType(arrayType))
>>          .fold(ARRAYCOPY)
>>          .permute(2)
>>          .cast(arrayType, arrayType)
>>          .identity();
>>
>> MethodHandle collector = transmuteArray.asCollector(Object[].class,
>> count).asType(source.dropParameterTypes(0,
>> index).changeReturnType(arrayType));
>>
>> return MethodHandles.collectArguments(target, index, collector);
>> ```
>>
>> Hopefully this is mostly readable. Basically I craft a chain of
>> handles that uses the normal Object[] collector and then simulates
>> what the pre-Jorn asCollector does: allocate the actual array we want
>> and arraycopy everything over. I figured this would be worth a try
>> since Jorn's comments on the PR hinted at the intermediate Object[]
>> going away for some collect forms. Unfortunately, reproducing the old
>> asCollector using MethodHandles does not appear to work any better...
>> or at least it still pales compared to a collector function.
>>
>> I am open to suggestions because my next attempt will probably be to
>> chain a series of folds together that populate the target array
>> directly, but it will be array.length deep. Not ideal and not a good
>> general solution.
>>
>> On Thu, Apr 1, 2021 at 6:44 PM Charles Oliver Nutter
>> <headius at headius.com> wrote:
>>> Very nice! I will have a look at the pull request and perhaps it 
>>> will lead me to a short-term work around as well.
>>>
>>> On Thu, Apr 1, 2021, 12:04 Jorn Vernee <jorn.vernee at oracle.com> wrote:
>>>> Hi Charlie,
>>>>
>>>> (Sorry for replying out of line like this, but I'm not currently
>>>> subscribed to the mlvm-dev mailing list, so I could not reply to your
>>>> earlier email thread directly.)
>>>>
>>>> I have fixed the performance issue with asCollector you reported [1],
>>>> and with the patch the performance should be the same/similar for any
>>>> array type (as well as fixing a related issue with collectors that 
>>>> take
>>>> more than 10 arguments). The patch is out for review here:
>>>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/3306__;!!GqivPVa7Brio!NeZJUPlqKG3_Z1yUBVS5cX1Gtz0pQMuFcTiR0PMhz42KSgmJpMNj8zkru_YUYkM-$ 
>>>>
>>>>
>>>> Cheers,
>>>> Jorn
>>>>
>>>> [1] : https://bugs.openjdk.java.net/browse/JDK-8264288
>>>>