RFR: JDK-8199216: Memory leak and quadratic layout time with nested nodes (hbox) and pseudo-class in style sheet [v3]

[John Hendrikx]

On Tue, 4 Apr 2023 09:31:12 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> I think I am missing something :)
>> 
>> The set that is passed in may indeed be immutable, but it may not be the set we have cached.  The memory savings only work when I then return the cached set; if I return the set you pass in (even if it is immutable) then we would still have large numbers of `Set<PseudoClass>` that are duplicates.
>> 
>> So, at a minimum, the `Set` that is passed in would need its hash code calculated so I can check if it is the cached version, or otherwise return the cached version.
>> 
>> I could cache the hash code for the sets that are returned here (they are immutable sets returned by `Set.copyOf` -- I checked their code, and they don't cache the hash codes).  That would however only help if you often pass in a set that is already cached, to find out if you got the same one.  Perhaps this happens a lot, I could test that.
>> 
>> Note that `HashMap` already caches hash codes for things that are part of the map, so only the hash code of the input needs to be calculated on this call.
>
> About allocation free: if the `Set` you pass in is immutable, and it is has a cached equivalent (or it is the same one), then no allocations occur (calculating the hash code does not do allocations, unless someone overrides hash code to do so) -- `copyOf` will not allocate anything if the `Set` was immutable already.
> 
> Allocations only occur in two cases:
> - When the set you pass in is not immutable, and it is a new set that wasn't cached yet.  A copy is made (allocation) and a `Map.Entry` is created (allocation)
> - When the set you pass in is immutable, but wasn't cached yet.  No copy is needed, but still a `Map.Entry` is created.
> 
> These allocations however will be only during application startup (when new combinations of pseudo classes are encountered) and the first time some state occurs in your application that results in a new pseudo class combination.  The amount of allocations over the life time of your application (from this code) is probably around 50-100.

Perhaps write some code to show what you mean here?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1076#discussion_r1156988973