On Wed, 17 Dec 2025 23:55:23 GMT, Cormac Redmond <[email protected]> wrote:
>> modules/javafx.base/src/main/java/javafx/collections/transformation/SortedList.java
>> line 280:
>>
>>> 278:
>>> 279: // Null out out-of-range array elements to avoid maintaining
>>> object references
>>> 280: for (int i = size; i < sorted.length; i++) {
>>
>> Don't we need to null the unused entries only when the `sorted` array
>> shrinks (i.e. when the `size` is reduced)?
>>
>> so all we do is to handle L259 case `if (c.wasRemoved())` since everywhere
>> else the references are null'ed (L379, L388)?
>
> Yes...and it works fine too if put inside the if(c.wasRemoved()) { } block
> too but if there are many other additions/permutations _also_ in the change
> set, then we might be wasting time/CPU nulling indexes that are going to be
> replaced anyway (i.e., thinking large lists). I felt it was cleaner/safer to
> just trim at the end of all changes applied in the while loop.
>
> Let me know what you think. It can be moved if you prefer. I have added
> comment anyway in the meantime, to mention why it's needed (i.e., only when
> shrinking). That's done now.
>
> The other methods you mentioned are only called when a comparator is set, by
> the way, there's no nulling in this method (updateUnsorted())
The problem with the current change is we are wasting time/CPU always, instead
of doing only what's strictly needed. Just imagine if the `sorted` array is
fairly large and contains only a few items - then you are overwriting many
`null` entries with `nulls` needlessly.
Besides, we have the exact number of entries to null out
(`c.getRemovedSize()`), which makes the life easier.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2000#discussion_r2629016001
