JDK java.util.concurrent.ConcurrentSkipListSet.equals(Object o) implementation efficiency

[Martin Buchholz]

Thanks for the suggestion!
I filed https://bugs.openjdk.java.net/browse/JDK-8181146

On Fri, May 26, 2017 at 7:21 AM, Tenghuan He <tenghuanhe at gmail.com> wrote:

> Hi, all
>
> I found that the equalsimplementation of
> java.util.concurrent.ConcurrentSkipListSet in JDK as follows:
>
> public boolean equals(Object o) {
>     // Override AbstractSet version to avoid calling size()
>     if (o == this)
>         return true;
>     if (!(o instanceof Set))
>         return false;
>     Collection<?> c = (Collection<?>) o;
>     try {
>         return containsAll(c) && c.containsAll(this);
>     } catch (ClassCastException unused) {
>         return false;
>     } catch (NullPointerException unused) {
>         return false;
>     }}
>
>
> Since ConcurrentSkipListSet extends NavigableSet which extends SortedSet,
> the following equalsimplementation seems more reasonable for a compare
> object which also extends SortedSet.
>
> public boolean myEquals(Object o) {
>         // Override AbstractSet version to avoid calling size()
>         if (o == this)
>             return true;
>         if (!(o instanceof Set))
>             return false;
>         if (o instanceof SortedSet) {
>             Iterator iter1 = ((SortedSet) o).iterator();
>             Iterator iter2 = iterator();
>
>             while (iter1.hasNext() && iter2.hasNext()) {
>                 if (!iter1.next().equals(iter2.next())) {
>                     return false;
>                 }
>             }
>             return !(iter1.hasNext() || iter1.hasNext());
>         }
>         Collection<?> c = (Collection<?>) o;
>         try {
>             return containsAll(c) && c.containsAll(this);
>         } catch (ClassCastException unused) {
>             return false;
>         } catch (NullPointerException unused) {
>             return false;
>         }
>     }
>
> A simple test has shown the performance improvement
>
> public class Test {
>     public static void main(String[] args) {
>         ConcurrentSkipListSet<Integer> set1 = new
> ConcurrentSkipListSet<Integer>();
>         ConcurrentSkipListSet<Integer> set2 = new
> ConcurrentSkipListSet<Integer>();
>
>         for (int i = 0; i < 10000000; i++) {
>             set1.add(i);
>             set2.add(i);
>         }
>
>         long ts = System.currentTimeMillis();
>         System.out.println(set1.equals(set2));
>         System.out.println(System.currentTimeMillis() - ts);
>
>         ts = System.currentTimeMillis();
>         System.out.println(myset1.myEquals(myset2));
>         System.out.println(System.currentTimeMillis() - ts);
>     }}
>
> Output result
>
> true2713true589
>
>
> Any ideas? (Related question on SO:
> https://stackoverflow.com/questions/44198103/jdk-java-util-concurrent-
> concurrentskiplistset-equalsobject-o-implementation-e
>
> Thanks & Best Regards
>
> Tenghuan He
>