On Fri, 3 Dec 2021 14:41:11 GMT, Ajit Ghaisas <aghai...@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
