gautamworah96 commented on a change in pull request #179: URL: https://github.com/apache/lucene/pull/179#discussion_r698080163
########## File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java ########## @@ -351,12 +348,142 @@ public FacetLabel getPath(int ordinal) throws IOException { } synchronized (categoryCache) { - categoryCache.put(catIDInteger, ret); + categoryCache.put(ordinal, ret); } return ret; } + private FacetLabel[] getPathFromCache(int... ordinals) { + FacetLabel[] facetLabels = new FacetLabel[ordinals.length]; + // TODO LUCENE-10068: can we use an int-based hash impl, such as IntToObjectMap, + // wrapped as LRU? + synchronized (categoryCache) { + for (int i = 0; i < ordinals.length; i++) { + facetLabels[i] = categoryCache.get(ordinals[i]); + } + } + return facetLabels; + } + + /** + * Checks if the ordinals in the array are >=0 and < {@code + * DirectoryTaxonomyReader#indexReader.maxDoc()} + * + * @param ordinals Integer array of ordinals + * @throws IllegalArgumentException Throw an IllegalArgumentException if one of the ordinals is + * out of bounds + */ + private void checkOrdinalBounds(int... ordinals) throws IllegalArgumentException { + for (int ordinal : ordinals) { + if (ordinal < 0 || ordinal >= indexReader.maxDoc()) { + throw new IllegalArgumentException( + "ordinal " + + ordinal + + " is out of the range of the indexReader " + + indexReader.toString() + + ". The maximum possible ordinal number is " + + (indexReader.maxDoc() - 1)); + } + } + } + + /** + * Returns an array of FacetLabels for a given array of ordinals. + * + * <p>This API is generally faster than iteratively calling {@link #getPath(int)} over an array of + * ordinals. It uses the {@link #getPath(int)} method iteratively when it detects that the index + * was created using StoredFields (with no performance gains) and uses DocValues based iteration + * when the index is based on BinaryDocValues. Lucene switched to BinaryDocValues in version 9.0 + * + * @param ordinals Array of ordinals that are assigned to categories inserted into the taxonomy + * index + */ + @Override + public FacetLabel[] getBulkPath(int... ordinals) throws IOException { + ensureOpen(); + checkOrdinalBounds(ordinals); + + int ordinalsLength = ordinals.length; + FacetLabel[] bulkPath = new FacetLabel[ordinalsLength]; + // remember the original positions of ordinals before they are sorted + int[] originalPosition = new int[ordinalsLength]; + Arrays.setAll(originalPosition, IntUnaryOperator.identity()); + + getPathFromCache(ordinals); + + /* parallel sort the ordinals and originalPosition array based on the values in the ordinals array */ + new InPlaceMergeSorter() { + @Override + protected void swap(int i, int j) { + int x = ordinals[i]; + ordinals[i] = ordinals[j]; + ordinals[j] = x; + + x = originalPosition[i]; + originalPosition[i] = originalPosition[j]; + originalPosition[j] = x; + } + + @Override + public int compare(int i, int j) { + return Integer.compare(ordinals[i], ordinals[j]); + } + }.sort(0, ordinalsLength); + + int readerIndex; + int leafReaderMaxDoc = 0; + int leafReaderDocBase = 0; + LeafReader leafReader; + LeafReaderContext leafReaderContext; + BinaryDocValues values = null; + List<Integer> uncachedOrdinalPositions = new ArrayList<>(); + + for (int i = 0; i < ordinalsLength; i++) { + if (bulkPath[originalPosition[i]] == null) { + /* + If ordinals[i] >= leafReaderDocBase + leafReaderMaxDoc then we find the next leaf that contains our ordinal. + Remember: ordinals[i] operates in the global ordinal space and hence we add leafReaderDocBase to the leafReaderMaxDoc + (which is the maxDoc of the specific leaf) + */ + if (values == null || ordinals[i] >= leafReaderDocBase + leafReaderMaxDoc) { + + readerIndex = ReaderUtil.subIndex(ordinals[i], indexReader.leaves()); + leafReaderContext = indexReader.leaves().get(readerIndex); + leafReader = leafReaderContext.reader(); + leafReaderMaxDoc = leafReader.maxDoc(); + leafReaderDocBase = leafReaderContext.docBase; + values = leafReader.getBinaryDocValues(Consts.FULL); + + /* + If the index is constructed with the older StoredFields it will not have any BinaryDocValues field and will return null + */ + if (values == null) { Review comment: Sure. I'll create it after this PR is merged. Otherwise, it just feels weird to create an issue for a fix in a piece of code that has not been merged. The fix should be trivial actually. We can just use the class variable `useOlderStoredFieldsIndex` from `TaxonomyDirectoryWriter` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org