On Fri, 20 Mar 2026 22:17:50 GMT, Marius Hanl <[email protected]> wrote:

>> modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxTest.java
>>  line 610:
>> 
>>> 608:     }
>>> 609: 
>>> 610:     @Test
>> 
>> +1 for adding tests!
>> 
>> However, the new tests do not test the issue being fixed in this PR, since 
>> they don't add 1000s of items and no timing checks.
>> 
>> Speaking of timing checks, how do we meaningfully and reliably measure the 
>> timing in a test?
>> 
>> For example, in this case we could measure adding 100 items, then set a 
>> threshold at N*10 for 1000 items where N is 2 or maybe 3 to account for 
>> slower access time with large number of items.  It is still unclear whether 
>> the test will be reliable enough given different platforms, architectures, 
>> L2/L3 CPU caches etc.
>> What do you think?
>
> I don't think there is a good way to test that. In the past, we instead made 
> sure that all the branches are covered by tests that were optimized. 
> We could probably measure the timing and have an arbitrary value we check 
> against. But I'm not sure this is really needed.

Hmmm, maybe we should check for the absence of quadratic complexity.

For example, if the T1 is the time to perform an operation for 100 items, then 
test that the time for 1000 items is below 0.8 * 10^2 * T1 (10 comes from 
1000/100).

The goal is to make sure some later, unrelated change does not cause loss of 
performance.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/2118#discussion_r2968231916

Reply via email to