Re: RFR: 8190427 : Test for JDK-8165198 fails intermittently because of GC

[Jim Laskey (Oracle)]

+1

> On Nov 7, 2017, at 8:45 AM, Hannes Wallnöfer  
> wrote:
> 
> Please review:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8190427
> Webrev: http://cr.openjdk.java.net/~hannesw/8190427/webrev.02/
> 
> This started as simple bug fix but turned into a full refactoring of the code 
> that manages property switch points. However, I do think it is actually the 
> only reasonable way too fix this bug, with the side benefit that the code 
> becomes simpler.
> 
> The gist of this change is that PropertyMap no longer acts as property change 
> listener of parent maps. Instead, the switch points are managed and 
> invalidated directly in the PropertySwitchPoints class (renamed from 
> PropertyListeners). Thus, we no longer depend on PropertyMaps being kept 
> around, which was the root of the bug, and there’s one less level of 
> indirection. 
> 
> I also replaced the property change callback methods in PropertyMap with a 
> single propertyChanged method because it’s now the same code for all types of 
> property change.
> 
> I spent quite some time testing behaviour of the new code, and it’s actually 
> as good or slightly better than the old one in terms of switch points created 
> and invalidated. I’m a bit embarrassed I made this more complex than it had 
> to be the first time around.
> 
> Hannes

RFR: 8190427 : Test for JDK-8165198 fails intermittently because of GC

[Hannes Wallnöfer]

Please review:

Bug: https://bugs.openjdk.java.net/browse/JDK-8190427
Webrev: http://cr.openjdk.java.net/~hannesw/8190427/webrev.02/

This started as simple bug fix but turned into a full refactoring of the code 
that manages property switch points. However, I do think it is actually the 
only reasonable way too fix this bug, with the side benefit that the code 
becomes simpler.

The gist of this change is that PropertyMap no longer acts as property change 
listener of parent maps. Instead, the switch points are managed and invalidated 
directly in the PropertySwitchPoints class (renamed from PropertyListeners). 
Thus, we no longer depend on PropertyMaps being kept around, which was the root 
of the bug, and there’s one less level of indirection. 

I also replaced the property change callback methods in PropertyMap with a 
single propertyChanged method because it’s now the same code for all types of 
property change.

I spent quite some time testing behaviour of the new code, and it’s actually as 
good or slightly better than the old one in terms of switch points created and 
invalidated. I’m a bit embarrassed I made this more complex than it had to be 
the first time around.

Hannes