On Wed, 8 Jul 2026 22:30:08 GMT, Viktor Klang <[email protected]> wrote:
>> Agreed. Thanks for pointing out that.
>> I've added a separate fast path for exact `PriorityQueue` sources.
>
> I haven't thought too deeply about this, but I presume you're targeting
> something like the following (untested):
>
>
> @Override
> public boolean addAll(Collection<? extends E> c) {
> if (getClass() == PriorityQueue.class && size == 0) { // fastpath
> if (Objects.requireNonNull(c) == this) // disallow null origins
> and self-concatenation
> throw new IllegalArgumentException();
>
> Object[] es;
> int len;
>
> if (c.isEmpty() || (es = c.toArray()) == null || (len =
> es.length) == 0)
> return false; // no TOCTOU problem calling c.isEmpty() as it
> is an early exit without modification
>
> final Class<?> cType = c.getClass();
>
> if (cType == PriorityQueue.class || cType == ArrayList.class)
> ; // origin produces a trusted array
> else
> es = Arrays.copyOf(es, len); // avoid TOCTOU
>
> if (cType == PriorityQueue.class)
> ; // origin already disallows nulls
> else {
> for (var e : es)
> Objects.requireNonNull(e); // reject any nulls
> }
>
> if (cType == PriorityQueue.class &&
> ((PriorityQueue<?>)c).comparator == comparator)
> ; // origin already has items in correct order
> else
> heapify(es, len, comparator); // ensure correct order
>
> ++modCount;
> queue = ensureNonEmpty(es);
> size = len;
> return true;
> }
> else
> return super.addAll(c); // fallback to default impl
> }
Thanks for the suggestion.
Sorry for the repeated churn here. I think I was being too conservative in the
previous version.
I have reworked it along those lines. How does this version look to you?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31701#discussion_r3547883456
