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

[Jorn Vernee]

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