On Fri, 3 Dec 2021 10:20:43 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:

>> Johan Vos has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Move functionality in the setCellCount() to the invalidated block.
>>   Some hard numbers used in tests (counters for evaluations) were changed 
>> because of this.
>>   Instead of relying on hard values, I modified the failing was into 
>> relative ones.
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java
>  line 1124:
> 
>> 1122:                 Platform.runLater(() -> {
>> 1123:                     Toolkit.getToolkit().firePulse();
>> 1124:                     assertTrue(rt_35395_counter < 7);
> 
> I see that you have modified assertions to use "lesser than" some expected 
> value. This may hide some incorrect test outcomes.
> Along with "lesser than" assertion, do you think we should add a "greater 
> than" assertion as well? This will have a range bounded expected value.
> This is applicable for all assertion changes in this PR.

The hard values have been changed a number of times, and I believe it is not 
really a good metric. 
What we want to ensure is that there is no functional regression and no 
performance penalty. The tests calculate the number of updateItem invocations 
and require those to be a strict number. With JDK-8089589, we fixed a number of 
issues that are related to the fact that the total size of the view (in case 
all cells are rendered with their preferred height) is not known. This is done 
by using an estimate of the total size. The estimate is 100% correct if we call 
updateItem for every item, but that would lead to a huge performance penalty in 
case the list contains a large amount of items with equal height.
Hence, there is a balance between minimizing the updateItem calls but still 
have a fair estimate of the total dimensions. Rather than requiring that the 
amount of calls should be a fixed number, I think it makes more sense to ensure 
that the amount of calls stays within reasonable boundaries, and is not growing 
exponentially when we add linearly more items, for example.

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

PR: https://git.openjdk.java.net/jfx/pull/683

Reply via email to