On Thu, 17 Mar 2022 21:10:14 GMT, Marius Hanl <mh...@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
