RFR: 8276553: ListView scrollTo() is broken after fix for JDK-8089589 [v2]

[Johan Vos]

On Fri, 3 Dec 2021 14:41:11 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> 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.
>
>> The hard values have been changed a number of times, and I believe it is not really a good metric.
> 
> I agree completely.
> 
>> 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.
> 
> My point is exactly this. I see that as part of this PR, you have added the upper boundary (rather than a fixed number) for assertions. I am asking whether we need a lower boundary as well?

I don't think we need a lower boundary. A value of 0 is not a bad thing, as long as the functional tests are ok.
The lowest possible value is implicitly determined by the usecase, which is covered by the functional tests.

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

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