Implement method to bulk add all collection elements to a PriorityQueue

[BaurzhanSakhariev]

Hi!

This PR doesn't have a link to a Jira Issue as according to contribution guidelines I can either create a patch and attach it to an issue on Jira or open a pull request - let me know if I misunderstood things and have to create a Jira issue anyway.

Description

PriorityQueue class doesn't have a constructor accepting array/collection and I created one for the cases, when heap elements are known in advance to save some time on queue creation: O(N) (heapify) vs Nlog(N) (calling add() in loop).

Solution

JDK PriorityQueue has a constructor, accepting Collection and it internally uses heapify - this PR is basically adaptation of this method (index is not 0 but 1 based and no need to check comparator presence).

Tests

I added a test creating a queue filled by random elements with the new method and checking its validity. Also added a potential usage (just took randomly first usage of add) and it didn't cause any failures.

Tried to run benchmarks using https://github.com/mikemccand/luceneutil but
got an error

src/python/competition.py", line 302
    raise ValueError(f'mode must be "cpu" or "heap" but got: {mode}')

may I ask is there any detailed guide on running Lucene benchmarks?

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Lucene maintainers access to contribute to my PR branch. (optional but recommended)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.

[dweiss]

Maybe add a method called addAll(Collection) which would be smart enough to detect when it can use this accelerated addition instead of element-by-element addition? I'm not sure an additional constructor is needed here.

[BaurzhanSakhariev]

Maybe add a method called addAll(Collection) which would be smart enough to detect when it can use this accelerated addition instead of element-by-element addition? I'm not sure an additional constructor is needed here.

Good point with Collection, thanks. No need to convert list to array if we don't use it directly anyway (because of 1-index shift) - can rather copy from collection.

It's always possible to build a heap if elements are provided and comparable - in JDK they check what kind of collection is provided and take comparator from it - but here we have lessThan. My understanding is that user decides whether it's possible to use faster addition - one of use cases when add() are not interrupted by pop() or any other structure changing operations, i.e when we build first and only then do something.

I added a constructor as other option would be to use existing ones and then do smth like addAll(items) - but I wanted to avoid usage of default constructor, filling it with sentinel null objects. But I actually found an issue in my constructor - need to add maxSize >= ArrayUtil.MAX_ARRAY_LENGTH check there, thanks!

Would it it be fine if I keep the constructor but adjust it to copy directly from Collection and add a size check?

[dweiss]

I wanted to avoid usage of default constructor, filling it with sentinel null objects

In all honesty, this is probably not something you can ever, ever notice when you compare the cost of building the heap with clearing a chunk of memory. And it makes the API simpler in my opinion. But sure, you can keep the constructor.

[BaurzhanSakhariev]

Fair point, I changed it to addAll. Also added a case to add into partially filled queue - just realized that you probably meant this use case and I misunderstood it.

[dweiss]

Looks good to me, thank you! Let's wait for checks to pass. Please add CHANGES.txt entry under the corresponding version (main or 9x, if you'd like this backported).

[dweiss]

I've added a proper issue number (https://issues.apache.org/jira/browse/LUCENE-10494) and tweaked minor details of the implementation (exception types thrown in addAll and add were different). I think it's good to go.

[dweiss]

Thank you!

[BaurzhanSakhariev]

Thanks!