Re: Somewhere in this constructor of PriorityBlockingQueue is hard to understand

Jason Mehrens via Concurrency-interest <[email protected]> Tue, 4 Aug 2020 14:13:17 +0000
Newsgroups gmane.comp.java.jsr.166-concurrency
Message-ID <DM5PR1801MB20741DA92E1844AE4792A709834A0@DM5PR1801MB2074.namprd18.prod.outlook.com>
Liu,

1. Instanceof allows subclasses which could be coded to break the assumptions.  GetClass means this exact implementation for which the source code is known.
2. Collection.toArray() requires type Object[] but sometimes implementations are broken.  See: https://bugs.openjdk.java.net/browse/JDK-6260652 and https://bugs.openjdk.java.net/browse/JDK-8160406
3. PQ must not contain nulls.  If there is a comparator and the collection was not exactly of type PQ then that collection may contain null which needs to be screened.  If the size is one then the collection may contain null because it may not have been checked against another element (including itself). See: https://bugs.openjdk.java.net/browse/JDK-5045147  If the size is greater than one and there is no comparator heapify() will catch the null element.

Jason


________________________________________
From: Concurrency-interest <[email protected]> on behalf of Liu via Concurrency-interest <[email protected]>
Sent: Tuesday, August 4, 2020 7:10 AM
To: Concurrency-interest
Subject: [concurrency-interest] Somewhere in this constructor of PriorityBlockingQueue is hard to understand

In this constructor of PriorityBlockingQueue, some codes are hard to understand.

    public PriorityBlockingQueue(Collection<? extends E> c) {
        this.lock = new ReentrantLock();
        this.notEmpty = lock.newCondition();
        boolean heapify = true; // true if not known to be in heap order
        boolean screen = true;  // true if must screen for nulls
        if (c instanceof SortedSet<?>) {
            SortedSet<? extends E> ss = (SortedSet<? extends E>) c;
            this.comparator = (Comparator<? super E>) ss.comparator();
            heapify = false;
        }
        else if (c instanceof PriorityBlockingQueue<?>) {
            PriorityBlockingQueue<? extends E> pq =
                (PriorityBlockingQueue<? extends E>) c;
            this.comparator = (Comparator<? super E>) pq.comparator();
            screen = false;
            if (pq.getClass() == PriorityBlockingQueue.class) // first one
                heapify = false;
        }
        Object[] a = c.toArray();
        int n = a.length;

        if (a.getClass() != Object[].class)  // second one
            a = Arrays.copyOf(a, n, Object[].class);
        if (screen && (n == 1 || this.comparator != null)) {  //third one
            for (int i = 0; i < n; ++i)
                if (a[i] == null)
                    throw new NullPointerException();
        }
        this.queue = a;
        this.size = n;
        if (heapify)
            heapify();
    }

1. When "pq.getClass() == PriorityBlockingQueue.class" Expression holds,
    Then heapify operation is not need to do. Isn't "c instanceof PriorityBlockingQueue<?>"
    enough to decide?

2. Why must make the actual type of array is Object[].class?

3. I just don't understand  this "&& (n == 1 || this.comparator != null)". Why is it written like this?

In newest JDK, http://hg.openjdk.java.net/jdk/jdk15/file/d2c6eb3b2c8d/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java#l242 ,
this constructor of PriorityBlockingQueue is same.



--------------------------------------------------------------------------------
Regards
Liu
_______________________________________________
Concurrency-interest mailing list
[email protected]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest