On Mon, 18 May 2026 18:57:52 GMT, Marius Hanl <[email protected]> wrote:
>> modules/javafx.base/src/main/java/javafx/collections/transformation/FilteredList.java
>> line 138:
>>
>>> 136: @Override
>>> 137: protected void sourceChanged(Change<? extends E> c) {
>>> 138: ensureSize(getSource().size());
>>
>> are you sure this is needed?
>>
>> the tests in the two JBS tickets pass without it, `addRemove()` invokes this
>> method in ~L239 (then calls it again with size+1 in L273). `update()` calls
>> it in ~L286, and `permutate()` does not need it.
>>
>> perhaps `addRemove()` can be further optimized so as not to call
>> `ensureSize()` twice?
>
> `update()` and `permutate()` do not call it anymore. I relocated them into
> one single location: Here.
> So functionality wise, it did not change.
>
> We could probably remove it, as we will catch any grow on size in
> `addRemove()`. But it might hurt performance.
> If we have a `FilteredList` with a size 1 and add 10_000 elements, we will
> call `ensureSize` a lot of times, which will lead to a degraded performance.
>
> But since we call it once here, we will immediately grow to the worst case
> size - if nothing need to be filtered and MAY only grow it in `addRemove()`
> when we temporarily have more elements due to processing the changes one by
> one.
I might be misunderstanding something.
1. Is `ensureSize()` L138 needed for `permutate()` ?
2. `addRemove()` calls `ensureSize()` twice (L138 and L237). can it call it
only once?
3. `update()` does not change the filtered size, right? why is it calling
`ensureSize()` in L138?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2163#discussion_r3262311559
