RFR: 6356745: (coll) Add PriorityQueue(Collection, Comparator) [v6]

[Chen Liang]

On Thu, 4 Jan 2024 15:34:03 GMT, Jason Mehrens <duke at openjdk.org> wrote:

>>> > I think this ticket should focus on adding the new constructor as part of the API.
>>> 
>>> Okay. I would think the code would avoid heapify when the caller does foolish things with this API such as:
>>> 
>>> ```
>>> SortedSet ss = filledSortedSet();
>>> PriorityQueue pq1 = new PriorityQueue(ss.comparator(), ss);
>>> 
>>> PriorityQueue pq = filledPriorityQueue();
>>> PriorityQueue pq2 = new PriorityQueue(pq.comparator(), pq);
>>> ```
>>> 
>>> I assume this constructor is going to be added to TreeSet, PriorityBlockingQueue, and ConcurrentSkipListSet?
>> 
>> why do you think the code would avoid heapify? `initFromCollection` method will be called regardless of the type of the collection passed, which will heapify the queue.
>> 
>> regarding adding the constructor to the other types mentioned, I believe I can be done, probably as part of a different ticket.
>
>> why do you think the code would avoid heapify? `initFromCollection` method will be called regardless of the type of the collection passed, which will heapify the queue.
> 
> I simply mean to point out:
> 
> 1.  That if the given comparator and the sortedset/priority queue comparator are the same then the call to heapify should be avoided as this is what is done in the [copy constructor.](https://github.com/openjdk/jdk/blob/139abc453f9eeeb4763076298ec44573ab94a3b6/src/java.base/share/classes/java/util/PriorityQueue.java#L196)
> 2. This new API has an anti-pattern not present with the other constructors.

I think it's that @jmehrens arguing that the "set of constructors" should be settled and there shouldn't be more or less, like

> This new API has an anti-pattern not present with the other constructors.

In all honesty, our collection API docs is massively outdated, that we haven't even documented SequencedCollection since JDK 21; those "constructor parity" arguments are relics of the age of serialization and can be safely discarded.

> I assume this constructor is going to be added to TreeSet, PriorityBlockingQueue, and ConcurrentSkipListSet?

Feel free to. If you submit a patch, we are more than glad to review it. An overhaul ("anti-pattern" in @jmehrens' words) caused by doing the right thing is not a reason not to do so.

-----------------

@archiecobbs You are right. I will create a CSR for this patch. Hope the original author can come back (just issue `/open` in a comment and bot will reopen PR), otherwise we can probably add such comparator+copy constructors to all these ordered collections.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17045#issuecomment-2234768805