RFR: 8283346: Optimize observable ArrayList creation in FXCollections [v4]

[Marius Hanl]

On Thu, 30 Jun 2022 19:36:34 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> Looks good. I left a few small comments.
> 
> I ran some benchmarks and I can reproduce the performance improvement both in the array (varargs) and the collection variants of the method.

Thanks for the review and verifying the improvement.
I implemented your suggested changes.

> modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line 351:
> 
>> 349:     public static <E> ObservableList<E> observableArrayList(E... items) {
>> 350:         ArrayList<E> backingList = new ArrayList<>(items.length);
>> 351:         backingList.addAll(Arrays.asList(items));
> 
> Unless I'm missing something again, this can be written as
> `ArrayList<E> backingList = new ArrayList<>(Arrays.asList(items));`
> 
> I created benchmarks based on yours, and there is no significant difference between these two (their scores are within the error ranges of each other).
> 
> I don't mind leaving it as is if you think it's clearer, I personally prefer the 1 line variant.

Thanks. I totally missed that, I prefer the 1 line variant as well.

> modules/javafx.base/src/test/java/test/javafx/collections/FXCollectionsTest.java line 47:
> 
>> 45:     @Test
>> 46:     public void testCreateObservableArrayListFromArray() {
>> 47:         ObservableList<String> observableList = FXCollections.observableArrayList("1", "2", "3");
> 
> I would add a `null` element (like `"1", "2", null`) to test that the method accepts `null`s, especially after I fell for it myself.

Good idea, will do.

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

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