epotyom commented on code in PR #12769:
URL: https://github.com/apache/lucene/pull/12769#discussion_r1383719688

##########
lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestDirectoryTaxonomyReader.java:
##########
@@ -476,6 +479,86 @@ public void testOpenIfChangedReplaceTaxonomy() throws 
Exception {
     src.close();
   }
 
+  private void assertGettingOrdinals(

Review Comment:
   There is `testCallingBulkPathReturnsCorrectResult` test (I renamed it to 
`testGetPathAndOrdinalsRandomMultithreading`) which runs multiple threads. It 
used to only call getBulkPath, I changed it to use `assertPathsAndOrdinals`, 
which means that each thread somewhat randomly calls `getPath`, `getBulkPath`, 
`getOrdinal` and the new `getBulkOrdinals`. But maybe we can improve it:
   
   1. It creates an array of 1K random facets, and the cache size 
`DEFAULT_CACHE_VALUE` is 4K, so there are no evictions AFAIU. I can change the 
array size to something like 10K to sometimes cause evictions.
   2. I'm not sure if calling all 4 methods in test threads is the right thing 
to do as it reduces chances for "collisions" e.g. calling getBulkPath and 
getBulkOrdinals in parallel is unlikely to reveal any issues because these 2 
methods use different cache instances, etc.  I think I can split into 2 tests: 
one to randomly call getPath+getBulkPath in multiple threads, and another one 
to do the same for getOrdinal+getBulkOrdinals?



##########
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java:
##########
@@ -201,6 +201,26 @@ public ChildrenIterator getChildren(final int ordinal) 
throws IOException {
    */
   public abstract int getOrdinal(FacetLabel categoryPath) throws IOException;
 
+  /**
+   * Returns the ordinals of the categories given as a path. The ordinal is 
the category's serial
+   * number, an integer which starts with 0 and grows as more categories are 
added (note that once a
+   * category is added, it can never be deleted).
+   *
+   * <p>The implementation in {@link
+   * org.apache.lucene.facet.taxonomy.directory.DirectoryTaxonomyReader} is 
generally faster than
+   * iteratively calling {@link #getOrdinal(FacetLabel)}
+   *
+   * @return array of the category's' ordinals or {@link #INVALID_ORDINAL} if 
the category wasn't
+   *     found.
+   */
+  public int[] getBulkOrdinals(FacetLabel... categoryPath) throws IOException {
+    int[] ords = new int[categoryPath.length];

Review Comment:
   Will do!



-- 
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

Reply via email to