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