RFR: 8283063: Optimize Observable{List/Set/Map}Wrapper.retainAll/removeAll [v3]

[Michael Strauß]

On Sat, 1 Apr 2023 07:32:20 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
>> 
>>  - Merge branch 'master' into fixes/JDK-8283063
>>    
>>    # Conflicts:
>>    #	modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java
>>    #	modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableMapWrapper.java
>>    #	modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableSequentialListWrapper.java
>>    #	modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableSetWrapper.java
>>  - address review comments
>>  - refactored removeAll/retainAll optimizations
>>  - Optimize removeAll/retainAll for Observable{List/Set/Map}Wrapper
>>  - Failing test
>
> modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableMapWrapper.java line 326:
> 
>> 324:             if (backingMap.isEmpty()) {
>> 325:                 return false;
>> 326:             }
> 
> Passing `null` is always an error, and I think here you still need to throw an NPE first when `c` is `null`, even if the backing map is empty.  From `AbstractColection` for example:
> 
>     public boolean retainAll(Collection<?> c) {
>         Objects.requireNonNull(c);
> 
>         ...
> 
> I realize this was already incorrect, so perhaps out of scope for your PR.

I've fixed problems like this and added comments in all places where a null check is important.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/751#discussion_r1155153443