RFR: 8246745: ListCell/Skin: misbehavior on switching skin

[Kevin Rushforth]

On Tue, 16 Jun 2020 10:07:22 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>>> hmm, do you mean a list with height of one cell only?
>> 
>> Yes, List with one cell height.
>> In continuation to my previous comment, looks like 8 of the first 23 calls when window is displayed for the first time
>> are NOT for the one visible cell. These 8 calls are for some intermediate cell. I added a single counter for all three
>> `computeXXHeight` methods and verified with a list of height of one cell. When `fixedCellSize` is not set, total
>> combined number of calls for a cell are, 1)   15, when Stage is displayed for the first time.(+8 for an intermediate
>> cell, not sure why) 2)   5, when the list item is selected.
>> 3)   5, when the Window is resized.
>
> something similar as I'm seeing - that's good :) Uploaded my [play
> code](https://gist.github.com/kleopatra/37ac1bffa0e2ad7580cd55344237cdca) to gist: the idea is to do something (like
> f.i. moving the selection), then press f1 to log the calls to each cell (the skin has a final instance counter and
> counters for calling the compute methods).   As much fun as this is (really like to dig until I'm dirty all over :) -
> at the end of the day we'll need some measurements (doing these is not so much fun, still hoping something already
> available)

Normally we implement this sort of lazy evaluation (caching + invalidation) when there is a computation that is being
saved, or when a micro-benchmark shows that something is called 1000s of times (e.g., in the inner loop of some common
operation).

I don't see either being the case here, but it's possible I missed something. The method in question,
`getFixedCellSize()` just calls `ListView::getFixedCellSize` which just returns the value of a property. I suspect that
any performance hit would be down in the noise.

So I think the complexity of the existing mechanism isn't justified. Removing the listener as the PR proposes to do,
which is possible since the listener isn't being used to trigger an operation, seems like a win as well.

I am inclined to approve this fix even without performance measurements, since it seems unlikely they would show a
problem.

@arapte if you want to measure it further before you approve it, that would be fine.

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

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