You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by gs...@apache.org on 2022/01/12 18:23:25 UTC

[lucene] branch main updated: revert LUCENE-10355 (#597)

This is an automated email from the ASF dual-hosted git repository.

gsmiller pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/lucene.git


The following commit(s) were added to refs/heads/main by this push:
     new 8d9fa6d  revert LUCENE-10355 (#597)
8d9fa6d is described below

commit 8d9fa6dba195eb440c4e2ea1349102733c220f9c
Author: gf2121 <52...@users.noreply.github.com>
AuthorDate: Thu Jan 13 02:23:13 2022 +0800

    revert LUCENE-10355 (#597)
    
    Trying to find the source of taxo-facet performance regression. See also LUCENE-10374
    
    Co-authored-by: guofeng.my <gu...@bytedance.com>
---
 lucene/CHANGES.txt                                 |  2 -
 .../facet/taxonomy/FastTaxonomyFacetCounts.java    | 76 +++++++---------------
 .../lucene/facet/taxonomy/IntTaxonomyFacets.java   | 17 +----
 3 files changed, 27 insertions(+), 68 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index c591882..0e768b1 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -154,8 +154,6 @@ Optimizations
 
 * LUCENE-10346: Optimize facet counting for single-valued TaxonomyFacetCounts. (Guo Feng)
 
-* LUCENE-10350: Avoid some duplicate null check in facet counting for TaxonomyFacetCounts. (Guo Feng)
-
 * LUCENE-10356: Further optimize facet counting for single-valued TaxonomyFacetCounts. (Greg Miller)
 
 Changes in runtime behavior
diff --git a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FastTaxonomyFacetCounts.java b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FastTaxonomyFacetCounts.java
index 1399afe..1e7d831 100644
--- a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FastTaxonomyFacetCounts.java
+++ b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FastTaxonomyFacetCounts.java
@@ -84,27 +84,13 @@ public class FastTaxonomyFacetCounts extends IntTaxonomyFacets {
           ConjunctionUtils.intersectIterators(Arrays.asList(hits.bits.iterator(), valuesIt));
 
       if (singleValued != null) {
-        if (values != null) {
-          while (it.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            values[(int) singleValued.longValue()]++;
-          }
-        } else {
-          while (it.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            sparseValues.addTo((int) singleValued.longValue(), 1);
-          }
+        while (it.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
+          increment((int) singleValued.longValue());
         }
       } else {
-        if (values != null) {
-          while (it.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            for (int i = 0; i < multiValued.docValueCount(); i++) {
-              values[(int) multiValued.nextValue()]++;
-            }
-          }
-        } else {
-          while (it.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            for (int i = 0; i < multiValued.docValueCount(); i++) {
-              sparseValues.addTo((int) multiValued.nextValue(), 1);
-            }
+        while (it.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
+          for (int i = 0; i < multiValued.docValueCount(); i++) {
+            increment((int) multiValued.nextValue());
           }
         }
       }
@@ -113,8 +99,7 @@ public class FastTaxonomyFacetCounts extends IntTaxonomyFacets {
     rollup();
   }
 
-  private final void countAll(IndexReader reader) throws IOException {
-    assert values != null;
+  private void countAll(IndexReader reader) throws IOException {
     for (LeafReaderContext context : reader.leaves()) {
       SortedNumericDocValues multiValued =
           context.reader().getSortedNumericDocValues(indexFieldName);
@@ -123,41 +108,28 @@ public class FastTaxonomyFacetCounts extends IntTaxonomyFacets {
       }
 
       Bits liveDocs = context.reader().getLiveDocs();
-      NumericDocValues singleValued = DocValues.unwrapSingleton(multiValued);
 
+      NumericDocValues singleValued = DocValues.unwrapSingleton(multiValued);
       if (singleValued != null) {
-        if (liveDocs == null) {
-          while (singleValued.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            values[(int) singleValued.longValue()]++;
-          }
-        } else {
-          for (int doc = singleValued.nextDoc();
-              doc != DocIdSetIterator.NO_MORE_DOCS;
-              doc = singleValued.nextDoc()) {
-            if (liveDocs.get(doc)) {
-              values[(int) singleValued.longValue()]++;
-            }
+        for (int doc = singleValued.nextDoc();
+            doc != DocIdSetIterator.NO_MORE_DOCS;
+            doc = singleValued.nextDoc()) {
+          if (liveDocs != null && liveDocs.get(doc) == false) {
+            continue;
           }
+          increment((int) singleValued.longValue());
         }
-      } else {
-        if (liveDocs == null) {
-          while (multiValued.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) {
-            final int dvCount = multiValued.docValueCount();
-            for (int i = 0; i < dvCount; i++) {
-              values[(int) multiValued.nextValue()]++;
-            }
-          }
-        } else {
-          for (int doc = multiValued.nextDoc();
-              doc != DocIdSetIterator.NO_MORE_DOCS;
-              doc = multiValued.nextDoc()) {
-            if (liveDocs.get(doc)) {
-              final int dvCount = multiValued.docValueCount();
-              for (int i = 0; i < dvCount; i++) {
-                values[(int) multiValued.nextValue()]++;
-              }
-            }
-          }
+        continue;
+      }
+
+      for (int doc = multiValued.nextDoc();
+          doc != DocIdSetIterator.NO_MORE_DOCS;
+          doc = multiValued.nextDoc()) {
+        if (liveDocs != null && liveDocs.get(doc) == false) {
+          continue;
+        }
+        for (int i = 0; i < multiValued.docValueCount(); i++) {
+          increment((int) multiValued.nextValue());
         }
       }
     }
diff --git a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
index cdec3f1..3f1dc17 100644
--- a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
+++ b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
@@ -31,21 +31,10 @@ import org.apache.lucene.facet.TopOrdAndIntQueue;
 /** Base class for all taxonomy-based facets that aggregate to a per-ords int[]. */
 public abstract class IntTaxonomyFacets extends TaxonomyFacets {
 
-  /**
-   * Dense ordinal values.
-   *
-   * <p>We are making this and {@link #sparseValues} protected for some expert usage. e.g. It can be
-   * checked which is being used before a loop instead of calling {@link #increment} for each
-   * iteration.
-   */
-  protected final int[] values;
+  /** Per-ordinal value. */
+  private final int[] values;
 
-  /**
-   * Sparse ordinal values.
-   *
-   * @see #values for why protected.
-   */
-  protected final IntIntHashMap sparseValues;
+  private final IntIntHashMap sparseValues;
 
   /** Sole constructor. */
   protected IntTaxonomyFacets(