On Tue, 24 Mar 2026 18:06:37 GMT, Philemon Hilscher <[email protected]> wrote:
>> The problem was caused, because the items were added individually instead of >> batched. Here is a performance comparison created with a profiler. >> >> | Before | After | >> | --- | --- | >> | <img width="912" height="489" alt="image" >> src="https://github.com/user-attachments/assets/794314ef-09d6-49e1-bbbe-d2f1a5c97cf2"; >> />|<img width="922" height="497" alt="Screenshot 2026-03-20 202806" >> src="https://github.com/user-attachments/assets/e3a38fde-97dc-4889-a229-3539787c60e9"; >> />| >> >> The tests added here back up the functionality of the choice box itself >> since this is just a performance improvement. > > Philemon Hilscher has updated the pull request incrementally with one > additional commit since the last revision: > > 8303060: Improve set items and add change listener to tests > > Signed-off-by: Philemon Hilscher <[email protected]> Looks good, thank you all for your effort helping @plumstone with this PR! Adding 1,000 items to the ChoiceBox does not lock the UI (on macOS 26.3.1), 10,000 items do lock for 3-4 seconds, however this is perfectly acceptable since the ChoiceBox is a bad _choice_ with a large model anyway. FYI: I've added 1k and 10k options to the standalone monkey tester https://github.com/andy-goryachev-oracle/MonkeyTest modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxTest.java line 620: > 618: AtomicInteger changeCount = new AtomicInteger(); > 619: choiceBoxPopup.getItems() > 620: .addListener((ListChangeListener.Change<? extends > MenuItem> _) -> changeCount.incrementAndGet()); this is very minor, but I'd recommend this format - it's clearer and one can actually set a breakpoint inside the listener: choiceBoxPopup.getItems().addListener((ListChangeListener.Change<? extends MenuItem> _) -> { changeCount.incrementAndGet(); }); modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxTest.java line 646: > 644: .addListener((ListChangeListener.Change<? extends > MenuItem> _) -> changeCount.incrementAndGet()); > 645: > 646: box.getItems().addAll(IntStream.range(0, 15).boxed().map(integer > -> "Item " + integer).toList()); minor observation: maybe create a utility function for the code which is replicated more than three times? box.getItems().addAll(createItemList(0, 15)); ------------- Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/2118#pullrequestreview-4001584799 PR Review Comment: https://git.openjdk.org/jfx/pull/2118#discussion_r2983664110 PR Review Comment: https://git.openjdk.org/jfx/pull/2118#discussion_r2983723373
