On Tue, 4 Apr 2023 09:26:21 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> Couldn't you just add a fast path by calculating the hash code of the 
>> immutable set eagerly and storing it in a field? This entire method could 
>> probably be written to be allocation-free if the set passed into the method 
>> is already an immutable set.
>
> 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.

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

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

Reply via email to