RFR: 8246745: ListCell/Skin: misbehavior on switching skin [v2]

[Ambarish Rapte]

On Thu, 10 Sep 2020 17:10:41 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> 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.

The few additional computation would not cause any harm to performance. It just increases few computations in the
compute* methods. Just for sanity verified a program with ListView of 10000000. ListView gets scrolled just as smooth.
Approving.

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

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