> This fixes an IOOBE and even improves the performance / change aggregation.
> I suggest reading Jeanette's comment in the ticket, that helped a lot to 
> understand it.
> 
> There was a code path that broke change events. 
> Jeanette wondered as well in the comment why this exists at all, if this was 
> meant to be some kind of optimization?
> 
> What I found out is: Because of this code path, we actually got 3 change 
> events and the order was wrong. 
> What we got with the test from Jeanette: `subChanges: { [B] removed at 1, [D] 
> removed at 2, [C, E, F] removed at 1 }`
> 
> Removing the code path and just calling `insertRemoved(..)` directly will 
> correctly find an alreadx existing sub change with `findSubChange` and 
> aggregate the changes together. So now, we only get one event: `subChanges: { 
> [B, C, D, E, F] removed at 1 }`. No error and we use the smart aggregation 
> from `insertRemoved`.
> 
> My speculation why this code existed is that there may was a former bug, 
> similar like [JDK-8208758](https://bugs.openjdk.org/browse/JDK-8208758) 
> (#2117) where I also did a fix in the `insertRemoved(..)` method.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Marius Hanl has updated the pull request incrementally with one additional 
commit since the last revision:

  Another test

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

Changes:
  - all: https://git.openjdk.org/jfx/pull/2154/files
  - new: https://git.openjdk.org/jfx/pull/2154/files/664d66e6..3e51bdfb

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=2154&range=01
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=2154&range=00-01

  Stats: 34 lines in 1 file changed: 33 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jfx/pull/2154.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/2154/head:pull/2154

PR: https://git.openjdk.org/jfx/pull/2154

Reply via email to