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