On Thu, 3 Oct 2024 19:40:43 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ScrollPaneBehavior.java
>> line 88:
>>
>>> 86:
>>> 87: new InputMap.KeyMapping(new KeyBinding(HOME), e ->
>>> verticalHome(), this::isNotFocused),
>>> 88: new InputMap.KeyMapping(new KeyBinding(END), e ->
>>> verticalEnd(), this::isNotFocused),
>>
>> minor: this change creates a bunch of lambdas
>> suggestion: declare
>>
>> Predicate<KeyEvent> isNotFocused = (ev) -> {
>> return !getNode().isFocused();
>> };
>>
>> and pass that to each key mapping instead
>
> It's an interesting suggestion, but it is not needed. `javac` will already
> deduplicate these.
>
> You can even verify that this is the case. Use `javap` to decompile the
> class file with `javap -c <classname>`. In there, `invokedynamic` is used to
> represent the lambda's. It looks like this for example:
>
> 120: invokedynamic #54, 0 // InvokeDynamic
> #1:test:(Lcom/sun/javafx/scene/control/behavior/ScrollPaneBehavior;)Ljava/util/function/Predicate;
>
> Later on, you'll see another:
>
> 152: invokedynamic #54, 0 // InvokeDynamic
> #1:test:(Lcom/sun/javafx/scene/control/behavior/ScrollPaneBehavior;)Ljava/util/function/Predicate;
>
> What you can see here is that the same constant (# 54) is used to reference
> the method. So, there's no need to help the compiler here.
hmmm... when I set up a breakpoint in Eclipse in KeyMapping:785 (after it hits
the ScrollPaneBehavior<init>) the value for `interceptor` changes
interceptor= 0x0000007001340d50 (id=208)
interceptor= 0x00000070013411c0 (id=212)
interceptor= 0x0000007001341630 (id=216)
I think it's still creates a different lambda object. (I recall testing this
assumption with some unit test a while back).
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1582#discussion_r1786773525
