RFR: 8283346: Optimize observable ArrayList creation in FXCollections

[yosbits]

On Thu, 17 Mar 2022 21:10:14 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> This simple PR optimizes the observable `ArrayList` creation by using the ArrayList constructor/array size so that the underlying array will be initialized at the correct size which will speed up the creation as the array does not need to grow as a result of the `addAll` call.
> 
> I also added tests which will succeed before and after to verify that nothing got broken by this change.
> Also I made a benchmark test. Results:
> 
> | Benchmark | Mode| Cnt | Score | Error | Units
> | ------------- | ------------- | ------------- | ------------- | ------------- | ------------- |
> | ListBenchmark OLD  | thrpt | 25 | 722,842 | ± 26,93 | ops/s
> | ListBenchmark NEW | thrpt  | 25 | 29262,274 | ± 2088,712 | ops/s
> 
> <details><summary>Benchmark code</summary>
> 
> 
> import javafx.collections.FXCollections;
> import javafx.collections.ObservableList;
> import org.openjdk.jmh.annotations.Benchmark;
> import org.openjdk.jmh.annotations.Scope;
> import org.openjdk.jmh.annotations.Setup;
> import org.openjdk.jmh.annotations.State;
> import org.openjdk.jmh.annotations.TearDown;
> 
> import java.util.ArrayList;
> import java.util.List;
> 
> @State(Scope.Benchmark)
> public class ListBenchmark {
> 
>     List<String> strings;
> 
>     @Setup
>     public void setup() {
>         strings = new ArrayList<>();
>         for(int i = 0; i< 100000;i++) {
>             strings.add("abc: " + i);
>         }
>     }
> 
>     @TearDown
>     public void tearDown() {
>         strings = null;
>     }
> 
>     @Benchmark
>     public ObservableList<String> init() {
>         return FXCollections.observableArrayList(strings);
>     }
> }
> 
> 
> </details>

This PR appears to have two internal behavior changes.

A. Initial collection size (PR owner's description)
B. By adding the item directly to the backingList
Avoiding beginChange () --doAdd () --endChange ()

``` java
     @Override
     protected void doAdd (int index, E element) {
         if (elementObserver! = null)
             elementObserver.attachListener (element);
         backingList.add (index, element);
     }


Looking at the following code, it seems that there is no problem because attachListener () is called in the constructor.
We will need to make sure that the current unit tests cover B's changes.

ObservableListWrapper.java
``` java
    public ObservableListWrapper(List<E> list) {
        backingList = list;
        elementObserver = null;
    }

    public ObservableListWrapper(List<E> list, Callback<E, Observable[]> extractor) {
        backingList = list;
        this.elementObserver = new ElementObserver(extractor, new Callback<E, InvalidationListener>() {

            @Override
            public InvalidationListener call(final E e) {
                return new InvalidationListener() {

                    @Override
                    public void invalidated(Observable observable) {
                        beginChange();
                        int i = 0;
                        final int size = size();
                        for (; i < size; ++i) {
                            if (get(i) == e) {
                                nextUpdate(i);
                            }
                        }
                        endChange();
                    }
                };
            }
        }, this);
        final int sz = backingList.size();
        for (int i = 0; i < sz; ++i) {
            elementObserver.attachListener(backingList.get(i));
        }
    }

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

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