On Tue, 7 Jul 2026 16:20:26 GMT, Viktor Klang <[email protected]> wrote:

>> Daisuke Yamazaki has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - Refactor prepareElements remove redundant clazz argument
>>  - Add fast path for exact PriorityQueue in prepareElements
>>  - More efficient way
>
> src/java.base/share/classes/java/util/PriorityQueue.java line 309:
> 
>> 307:                     throw new NullPointerException();
>> 308:         return es;
>> 309:     }
> 
> What's the purpose of separating out initElementsFromArray and 
> prepareElements?
> 
> Also, does the array needs to get checked for null-objects if it originates 
> from a PriorityQueue?

Hello @viktorklang-ora
Thanks for the review.

> What's the purpose of separating out initElementsFromArray and 
> prepareElements?

The split is intended to preserve failure atomicity.

If we directly reuse the existing 
`initFromCollection`/`initElementsFromCollection`, `queue`/`size` are assigned 
before `heapify`.
However, `heapify` can throw, for instance when the elements are not mutually 
comparable.
In that order, a failed `addAll` could leave the receiver modified.

So the intent is to separate array preparation from committing the instance 
state: first prepare and heapify the local array, and only assign queue / size 
after that succeeds.

> Also, does the array needs to get checked for null-objects if it originates 
> from a PriorityQueue?

Good catch. No, it does not.

For an exact `PriorityQueue`, `null` elements cannot be present and its 
`toArray` implementation is trusted.
But subclasses can override `toArray`, so I would only add that fast path for 
`cClass == PriorityQueue.class` and keep the existing checks otherwise.

Thanks again.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31701#discussion_r3539481945

Reply via email to