LUCENE-9476 Add getBulkPath API for the Taxonomy index

[gautamworah96]

Description

In LUCENE-9450 we switched the Taxonomy index from Stored Fields to BinaryDocValues. In the resulting implementation of the getPath code, we create a new BinaryDocValues's values instance for each ordinal.
It may happen that we may traverse over the same nodes over and over again if the getPath API is called multiple times for ordinals in the same segment/with the same readerIndex.

This PR takes advantage of that fact by sorting ordinals and then trying to find out if some of the ordinals are present in the same segment/have the same readerIndex (by trying to advanceExact to the correct position and not failing) thereby allowing us to reuse the previous BinaryDocValues object.

Solution

Steps:

1. Sort all ordinals and remember their position so as to store the path in the correct position
2. Try to advanceExact to the correct position with the previously calculated readerIndex. If the operation fails, try to find the correct segment for the ordinal and then advanceExact to the desired position.
3. Store this position for future ordinals.

Tests

Added a new test for the API that compares the individual getPath results from ordinals with the bulk FacetLabels returned by the getBulkPath API

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 Solr maintainers access to contribute to my PR branch. (optional but recommended)
• I have developed this patch against the master branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Ref Guide (for Solr changes only).

[mikemccand]

Thanks @gautamworah96

Once we iterate to a solid PR I am very curious how this helps facets performance -- we can switch luceneutil over to this bulk API to test.

[shaie]

I am slowly getting back on the horse here 😄 , so this review focuses mainly on style ..

[mikemccand]

Thanks @gautamworah96 -- looking closer!

[mikemccand]

Oooh that is a great idea, and low-hanging fruit, and would greatly reduce the RAM usage for this cache.

I think DirectoryTaxonomyWriter also has such a cache that we could change to a native map.

Could you open a spinoff issue?

[mikemccand]

Thanks @gautamworah96, looks close!

[mikemccand]

Hmm, I'm confused -- wouldn't an older index have no BinaryDocValues field? So, values would be null, and we should fallback then?

This code should hit NullPointerException on an old index I think? How come our backwards compatibility test didn't expose this?

[mikemccand]

We will sometimes put ordinals back into the cache that were already there at the start of this method right? I guess that's harmless. Or, maybe we should move this up above? Then we can do it only for those ordinals that were not already cached?

[gautamworah96]

I think intuitively adding the ordinals back into the cache would not be a problem. This should also (theoretically) be faster than trying to get the lock again and again in a loop?

[mikemccand]

This should also (theoretically) be faster than trying to get the lock again and again in a loop?

Hmm, I'm confused: this code is already getting the lock inside a for loop? I guess we could move the synchronized outside of the for loop? Or, maybe javac is doing this for us already? But let's make it explicit, or, let's just merge this for loop with the one before (and keep acquiring the lock inside the for loop)? One big benefit of the latter approach is that if all of the ordinals were already cached (hopefully typically a common case), we do not need any locking, but with this approach, we still do.

[gautamworah96]

Once we iterate to a solid PR I am very curious how this helps facets performance -- we can switch luceneutil over to this bulk API to test.

Today both IntTaxonomyFacets and FloatTaxonomyFacets iteratively call getPath on all the top ordinals and then return the topChildren FacetLabels that the user wanted. With this API change, we could switch over IntTaxonomyFacets and FloatTaxonomyFacets to use this bulk API. All downstream children Facets such as FastTaxonomyFacetCount use this base getTopChildren function so all users will be able to benefit from this change by default.

Surprisingly,
Our luceneutil benchmark only tests the getTopChildren API so we should be able to see the performance change with the stock luceneutil package.