On Wed, 17 Dec 2025 23:27:47 GMT, Andy Goryachev <[email protected]> wrote:
>> Cormac Redmond has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8184166: add back newline at end of file, mistakenly removed in a previous
>> commit, to ensure no unexpected consequences
>
> 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. I should add a comment though, at the very least to
mention about it only solving the issue related to shrinking. I will do that
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())
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2000#discussion_r2629004227
