On Wed, 19 Apr 2023 12:51:27 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
> We probably need a couple pairs of eyes on this (and lots of testing) to > ensure no regressions. > > Are there any other cases where a recalculation might be needed? There are recalculations happening in a number of places already. The difficulty here is that there are often no clear rules about the expected outcome. For example, I filed JDK-8306447 with a PR #1099 . There is no explicit contract that states that when adding an element to the bottom of a list, there should be no "jumps" in the list (where "jump" is not well-defined either). Different developers might have different assumptions, and in case there is no explicit contract in the spec (of javadoc), it is hard to determine if something is a bug or changed behavior due to internal implementation changes. A number of changes in the VirtualFlow are done because there was sort of a consensus amongst developers that a specific desired behavior was not met. For those changes, test cases have been created that failed before and succeeded after the change. Those serve as regression tests now (but clearly not all possible cases are added as tests). One of the functional changes done before caused a large amount of recalculations that were irrelevant (if the size of the cells is not changed, there should be no recalculations in this part of the code -- which does not exclude that other recalculations might still be needed in case of changes in other parameters). Our current tests fail to detect this, as the tests discussed before don't check for performance changes. The patch for JDK-8306447 is unrelated to this PR (for JDK-8293836), but it adds yet another regression test. The more regression tests we have, the more confident we can be that performance improvements are not causing regression. Having said that, I absolutely agree that this needs lots of testing (and many eyes). ------------- PR Comment: https://git.openjdk.org/jfx/pull/1098#issuecomment-1514736060
