[11] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of() (original) (raw)

Claes Redestad claes.redestad at oracle.com
Thu Jan 18 17:51:51 UTC 2018

Hi,

noticed I had replied off-list to Peter by accident. Sorry for the unintended delay..

I think we can settle for the simplified equals (as previously outlined) along with an abstract hashCode to remind ourselves it needs to be overridden:

http://cr.openjdk.java.net/~redestad/8193128/open.06/

As for the race that's can make equals() return true when the other set is cleared after calling size() on it but before iterating over the collection, then I think that's behavior you could provoke with most of the non-j.u.c Set implementations in the JDK (which typically defer to AbstractSet::equals) - including the existing Set.of() implementation - so I consider it out of scope for this improvement.

Thanks!

/Claes

On 2018-01-09 13:27, Peter Levart wrote:

Hi Claes,

On 01/09/2018 02:17 AM, Claes Redestad wrote: If we took it a step further we could save us the trouble of throwing and swallowing NPEs in AbstractImmutableSet::equals when the other set contains null, which might be an optimization in that case:

@Override public boolean equals(Object o) { if (o == this) return true; if (!(o instanceof Set)) return false; Collection c = (Collection) o; if (c.size() != size()) return false; for (Object e : c) if (e == null || !contains(e)) return false; return true; } It might be rare enough that it's not worth bothering with, though. That's nicer in my opinion. What to do with ClassCastException from contains(e)? Perhaps just document that any future implementation that might throw it from contains() should also modify the equals() method and catch it there. And if you already do explicit iteration over elements, you could as well count them and do the size check with counted size at the end. Imagine the following scenario: ConcurrentHashMap m = new ChocurrentHashMap(); m.put(1, 1); m.put(2, 2); m.put(3, 3); Set s = Set.of("a", "b", "c"); Thread 1: s.equals(m.keySet()) Thread 2: m.clear(); It is surprising when Thread 1 encounters 'true' as the result of the equals() above which can happen when code checks for sizes separately. Iterating CHM.keySet() does not provide a snapshot of the content either, but counting iterated and compared elements nevertheless exhibits less surprising behavior. I don't know if it matters though. Regards, Peter

More information about the core-libs-dev mailing list