On Tue, 4 Apr 2023 09:31:12 GMT, John Hendrikx <jhendr...@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
