RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v18]

Kevin Rushforth kcr at openjdk.org
Fri Jul 8 13:07:09 UTC 2022


On Fri, 8 Jul 2022 10:16:33 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> John Hendrikx has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 27 additional commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into feature/fluent-bindings
>>  - Add null checks in Subscription
>>  - Update copyrights
>>  - Move private binding classes to com.sun.javafx.binding package
>>  - Add note to Bindings#select to consider ObservableValue#flatMap
>>  - Fix bug invalidation bug in FlatMappedBinding
>>    
>>    Also fixed a secondary issue where the indirect source of the binding
>>    was unsubscribed and resubscribed each time its value was recomputed.
>>    
>>    Add additional comments to clarify how FlatMappedBinding works.
>>    
>>    Added test cases for these newly discovered issues.
>>  - Fix typos in LazyObjectBinding
>>  - Rename observeInputs to observeSources
>>  - Expand flatMap javadoc with additional wording from Optional#flatMap
>>  - Add since tags to all new API
>>  - ... and 17 more: https://git.openjdk.org/jfx/compare/71632011...d66f2ba6
>
> Remember this program? :)
> 
>> ```
>> // Sample program that shows stub leakage:
>> public static void main(String[] args) throws InterruptedException {
>>   StringProperty x = new SimpleStringProperty();
>> 
>>   // Startup
>>   Thread.sleep(20000);
>> 
>>   for(int i = 0; i < 10000000; i++) {
>>     new SimpleStringProperty().bind(x);
>>   }
>> 
>>   // Bindings added
>>   System.gc();  // 400 MB of uncollectable stubs does not get GC'd
>>   Thread.sleep(20000);
>> 
>>   // Triggers "stub" clean-up (which fails miserably because of lots of array copying)
>>   x.set("A");
>> 
>>   System.gc();
>>   Thread.sleep(20000);
>> }
>> ```
>> 
>> ![image](https://user-images.githubusercontent.com/995917/177480797-825bd41c-f373-4318-953e-c1d7ef785e22.png)
> 
> I modified `ExpressionHelper` to use a custom `Collection` class that has `O(log N)` performance for the only things it uses (add at end, remove a specific listener, iteration). It has the exact same characteristics as `ArrayList`, that is, it is ordered, it allows duplicates and it maintains the exact positions when duplicates are part of the list, which means the call order of listeners is exactly the same (a `LinkedHashMap<Listener, counter>` would not have this characteristic).  Furthermore, it is very similar to an `ArrayList`, which means relatively low overhead (there is no `Entry` wrapper for each item in the "list") and good cache locality when iterating.
> 
> Performance looks now like this:
> 
> ![image](https://user-images.githubusercontent.com/995917/177971749-ab4e0b9e-81db-47f5-9e6b-24a45ef84021.png)
> 
> The price for this is increased memory use (you can see that having 10.000.000 binding stubs took around 400 MB before, and now it takes about twice that).  The extra memory use is only paid when there is more than 1 listener (as per how `ExpressionHelper` operates).

@hjohn If there are no more questions or concerns raised in the next few hours, you can integrate this PR. Please file the follow-up JBS bug to address the cleanup.

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

PR: https://git.openjdk.org/jfx/pull/675


More information about the openjfx-dev mailing list