On Wed, 8 Jun 2022 13:42:11 GMT, Jorn Vernee <jver...@openjdk.org> wrote:

> Looking at the code in MethodHandles::filterArguments, and this optimization. 
> It seems that for multiple filters we could potentially always generate just 
> a single new lambda form if we wanted, not just for repeated filters.
> 
> It would be similar to `LambdaFormEditor::makeRepeatedFilterForm`. Just need 
> to extend the species data with one new `L_TYPE` field for each unique 
> filter, generate a new getCombiner `Name` for each unique combiner, pass in 
> an array of combiner types instead of just a single one, and pass in an 
> additional array of ints that marks the combiner used by each argument 
> position. Then do similar wiring up as we do now, but calling the 
> corresponding combiner for each argument pos. Right?
> 
> (If I'm right about that, maybe that's even a more interesting direction to 
> explore, since it would benefit everyone, and the code in StringConcatFactory 
> could stay the same)

You're right, but I think such a transform would add quite a bit of complexity. 

Back when I added the `makeRepeatedFilterForm` optimization (for the benefit of 
`StringConcatFactory` 😄 ) I sought to strike a balance between the simple 
repeatedly-create-`filterArgumentForm`s transform we would do before and more 
complex forms that would remove more intermediates. I was also a bit concerned 
that such "shortcutting" transforms could mean we eliminate sharing, and the 
more complex we make things the harder it gets to reason about. Example:


...
mh1 = filterArgument(baseMH, 0, filter);
mh1 = filterArgument(mh1, 0, filter);

mh2 = filterArgument(baseMH, 0, filter, filter);

mh1.form() == mh2.form(); // false


The two different pathways end up with two different LFs, since 
`makeRepeatedFilterForm` shortcuts. In practice this might be a non-issue, but 
I figured it'd be nice if we could have a reasonable pathway to resolve 
mh1.form() and mh2.form() to the same thing in the future.

-------------

PR: https://git.openjdk.java.net/jdk/pull/9082

Reply via email to