On Thu, 28 Mar 2024 22:21:32 GMT, drmarmac <d...@openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java 
>> line 166:
>> 
>>> 164:             sm.startAtomic();
>>> 165: 
>>> 166:             final List<Integer> removed = new 
>>> ArrayList<>(c.getRemovedSize());
>> 
>> I wonder if we should add a check for 0 size here to bypass all this code 
>> and unnecessary object allocations if nothing is removed (same for added)
>
> We certainly could, or maybe use wasRemoved(), but I doubt there will be much 
> impact. I guess it's preferred to keep changes unrelated to the issue 
> minimal, so I'd leave it as it is if everyone's ok with that.

These short circuiting operations tend to be worth it only if it's a critical 
path. The GC will collect the allocations very efficiently these days. I didn't 
check what this code segment is used for, but unless it's looped somewhere, I 
doubt there will by any change.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1543793491

Reply via email to