RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

[mstr2]

On Mon, 5 Apr 2021 21:54:40 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> The internal BidirectionalBinding class implements bidirectional bindings for JavaFX properties. The design intent of this class is to provide specializations for primitive value types to prevent boxing conversions (cf. specializations of the Property class with a similar design intent).
>> 
>> However, the primitive BidirectionalBinding implementations do not meet the design goal of preventing boxing conversions, because they implement ChangeListener.
>> 
>> ChangeListener is a generic SAM interface, which makes it impossibe to invoke an implementation of ChangeListener::changed with a primitive value (i.e. any primitive value will be auto-boxed).
>> 
>> The boxing conversion happens, as with all ChangeListeners, at the invocation site (for example, in ExpressionHelper). Since the boxing conversion has already happened by the time any of the BidirectionalBinding implementations is invoked, there's no point in using primitive specializations of BidirectionalBinding after the fact.
>> 
>> This issue can be solved by having BidirectionalBinding implement InvalidationListener instead, which by itself does not incur a boxing conversion. Because bidirectional bindings are eagerly evaluated, the observable behavior remains the same.
>> 
>> I've filed a bug report with the same title.
>
> I don't see this or any similar bug filed in our incident tracker. Did the submission complete to the point where you have an internal tracking number?
> 
> Is there a measurable benefit in doing this? For example, do you have a benchmark of some sort that shows garbage generation has been reduced or performance has been improved?

Seems like I forgot to hit the send button on the webform. Here's the tracking number: 9069787.

I've used the following manual benchmark, which bidirectionally binds two properties and then produces a billion change notifications.
DoubleProperty property1 = new SimpleDoubleProperty();
DoubleProperty property2 = new SimpleDoubleProperty();
property2.bindBidirectional(property1);

long start = System.nanoTime();

for (int i = 0; i < 1000000000; ++i) {
    property1.set((i % 2 == 0) ? 12345.0 : 54321.0);
}

long end = System.nanoTime();

System.out.println("Time elapsed (ms): " + (end - start) / 1000000);

And these are the results I got (time elapsed, in milliseconds):

| | before | after |
| ------------|--------|-------|
| Test run 1: | 28608 | 9122 |
| Test run 2: | 27928 | 9065 |
| Test run 3: | 31140 | 9181 |
| Test run 4: | 28041 | 9127 |
| Test run 5: | 28730 | 9181 |

So in this synthetic benchmark, the new implementation has a 3x performance improvement compared to the old implementation.

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

PR: https://git.openjdk.java.net/jfx/pull/454