On Thu, 18 Dec 2025 00:03:52 GMT, Andy Goryachev <[email protected]> wrote:
>> 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.
I must be missing something. I don't see how that situation can occur, if
sorted is large but only contains a few items, then the source list must have
been large at one point, and shrunk, at which point to excess would be nulled,
once.
There's a break there to stop subsequent calls from needlessly nulling anything
out on subsequent changes.
On the other hand, putting inside the if statement means we're needlessly
removing elements if they're just about to be overwritten from other changes in
the same change set.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2000#discussion_r2629027195
