RFR: 8253086: Optimization of removeAll and retainAll of ObservableListWrapper

[yosbits]

On Mon, 21 Sep 2020 10:40:37 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8253086
>> 
>> ObservableListWrapper.java
>>  * public boolean removeAll(Collection<?> c)
>> * public boolean retainAll(Collection<?> c)
>> 
>> These two methods use BitSet, but it doesn't make sense.
>> By rewriting to the equivalent behavior that does not use BitSet, it is possible to reduce the CPU load in a wide range
>> of use cases.
>> The test is done with the following command.
>> 
>> * gradle base: test
>> * gradle controls: test
>
> modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java line 191:
> 
>> 189:         if (this.isEmpty() || c.isEmpty()) {
>> 190:             return false;
>> 191:         }
> 
> logic seems wrong: if c is empty this must be cleared
> 
> The error shows up in a failing test (but only when running all base.collections tests in Eclipse, not with gradle on
> the commandline)
> the test failure is in ObservableListWithExtractor.testUpdate_retainAll() which - besides exposing the error above -
> tells us two issues with the tests
> a) the normal (that is no extractor) test is incomplete (in not testing retainAll with empty array/collection)
> b) the test is not run with gradle because it doesn't adhere to naming conventions ;)

I realized I was wrong. Thank you. I would probably need to add a test for this case.

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

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