You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by kr...@apache.org on 2022/04/30 02:43:23 UTC

[lucene] branch main updated: LUCENE-10542: FieldSource exists implementations can avoid value retrieval (#847)

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

krisden 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 3063109d837 LUCENE-10542: FieldSource exists implementations can avoid value retrieval (#847)
3063109d837 is described below

commit 3063109d8375b7bd94873e7ac70c52ad57037773
Author: Kevin Risden <ri...@users.noreply.github.com>
AuthorDate: Fri Apr 29 22:43:16 2022 -0400

    LUCENE-10542: FieldSource exists implementations can avoid value retrieval (#847)
---
 lucene/CHANGES.txt                                 |  2 +
 .../function/valuesource/BytesRefFieldSource.java  | 35 ++++++-------
 .../function/valuesource/DoubleFieldSource.java    | 55 ++++++-------------
 .../function/valuesource/EnumFieldSource.java      | 51 +++++-------------
 .../function/valuesource/FloatFieldSource.java     | 49 +++++------------
 .../function/valuesource/IntFieldSource.java       | 51 +++++-------------
 .../function/valuesource/LongFieldSource.java      | 61 ++++++----------------
 7 files changed, 86 insertions(+), 218 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index ddf5e482faa..a9b8c4b0e29 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -128,6 +128,8 @@ Optimizations
 * LUCENE-8836: Speed up calls to TermsEnum#lookupOrd on doc values terms enums
   and sequences of increasing ords. (Bruno Roustant, Adrien Grand)
 
+* LUCENE-10542: FieldSource exists implementations can avoid value retrieval (Kevin Risden)
+
 Bug Fixes
 ---------------------
 * LUCENE-10477: Highlighter: WeightedSpanTermExtractor.extractWeightedSpanTerms to Query#rewrite
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java
index cd2e2257471..959a064c3fd 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/BytesRefFieldSource.java
@@ -45,35 +45,33 @@ public class BytesRefFieldSource extends FieldCacheSource {
     // To be sorted or not to be sorted, that is the question
     // TODO: do it cleaner?
     if (fieldInfo != null && fieldInfo.getDocValuesType() == DocValuesType.BINARY) {
-      final BinaryDocValues binaryValues = DocValues.getBinary(readerContext.reader(), field);
+      final BinaryDocValues arr = DocValues.getBinary(readerContext.reader(), field);
       return new FunctionValues() {
         int lastDocID = -1;
 
-        private BytesRef getValueForDoc(int doc) throws IOException {
+        @Override
+        public boolean exists(int doc) throws IOException {
           if (doc < lastDocID) {
             throw new IllegalArgumentException(
                 "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
           }
           lastDocID = doc;
-          int curDocID = binaryValues.docID();
+          int curDocID = arr.docID();
           if (doc > curDocID) {
-            curDocID = binaryValues.advance(doc);
-          }
-          if (doc == curDocID) {
-            return binaryValues.binaryValue();
-          } else {
-            return null;
+            curDocID = arr.advance(doc);
           }
-        }
-
-        @Override
-        public boolean exists(int doc) throws IOException {
-          return getValueForDoc(doc) != null;
+          return doc == curDocID;
         }
 
         @Override
         public boolean bytesVal(int doc, BytesRefBuilder target) throws IOException {
-          BytesRef value = getValueForDoc(doc);
+          BytesRef value;
+          if (exists(doc)) {
+            value = arr.binaryValue();
+          } else {
+            value = null;
+          }
+
           if (value == null || value.length == 0) {
             return false;
           } else {
@@ -110,12 +108,9 @@ public class BytesRefFieldSource extends FieldCacheSource {
 
             @Override
             public void fillValue(int doc) throws IOException {
-              BytesRef value = getValueForDoc(doc);
-              mval.exists = value != null;
+              mval.exists = exists(doc);
               mval.value.clear();
-              if (value != null) {
-                mval.value.copyBytes(value);
-              }
+              bytesVal(doc, mval.value);
             }
           };
         }
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/DoubleFieldSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/DoubleFieldSource.java
index 7c2e1b2c6bf..2d99b459774 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/DoubleFieldSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/DoubleFieldSource.java
@@ -25,8 +25,6 @@ import org.apache.lucene.queries.function.FunctionValues;
 import org.apache.lucene.queries.function.docvalues.DoubleDocValues;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.SortField.Type;
-import org.apache.lucene.util.mutable.MutableValue;
-import org.apache.lucene.util.mutable.MutableValueDouble;
 
 /**
  * Obtains double field values from {@link org.apache.lucene.index.LeafReader#getNumericDocValues}
@@ -52,55 +50,32 @@ public class DoubleFieldSource extends FieldCacheSource {
   public FunctionValues getValues(Map<Object, Object> context, LeafReaderContext readerContext)
       throws IOException {
 
-    final NumericDocValues values = getNumericDocValues(context, readerContext);
+    final NumericDocValues arr = getNumericDocValues(context, readerContext);
 
     return new DoubleDocValues(this) {
       int lastDocID;
 
-      private double getValueForDoc(int doc) throws IOException {
-        if (doc < lastDocID) {
-          throw new IllegalArgumentException(
-              "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
-        }
-        lastDocID = doc;
-        int curDocID = values.docID();
-        if (doc > curDocID) {
-          curDocID = values.advance(doc);
-        }
-        if (doc == curDocID) {
-          return Double.longBitsToDouble(values.longValue());
+      @Override
+      public double doubleVal(int doc) throws IOException {
+        if (exists(doc)) {
+          return Double.longBitsToDouble(arr.longValue());
         } else {
           return 0.0;
         }
       }
 
-      @Override
-      public double doubleVal(int doc) throws IOException {
-        return getValueForDoc(doc);
-      }
-
       @Override
       public boolean exists(int doc) throws IOException {
-        getValueForDoc(doc);
-        return doc == values.docID();
-      }
-
-      @Override
-      public ValueFiller getValueFiller() {
-        return new ValueFiller() {
-          private final MutableValueDouble mval = new MutableValueDouble();
-
-          @Override
-          public MutableValue getValue() {
-            return mval;
-          }
-
-          @Override
-          public void fillValue(int doc) throws IOException {
-            mval.value = getValueForDoc(doc);
-            mval.exists = exists(doc);
-          }
-        };
+        if (doc < lastDocID) {
+          throw new IllegalArgumentException(
+              "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
+        }
+        lastDocID = doc;
+        int curDocID = arr.docID();
+        if (doc > curDocID) {
+          curDocID = arr.advance(doc);
+        }
+        return doc == curDocID;
       }
     };
   }
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/EnumFieldSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/EnumFieldSource.java
index 20af9ec274f..a618513bafe 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/EnumFieldSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/EnumFieldSource.java
@@ -25,8 +25,6 @@ import org.apache.lucene.queries.function.FunctionValues;
 import org.apache.lucene.queries.function.ValueSourceScorer;
 import org.apache.lucene.queries.function.docvalues.IntDocValues;
 import org.apache.lucene.search.Weight;
-import org.apache.lucene.util.mutable.MutableValue;
-import org.apache.lucene.util.mutable.MutableValueInt;
 
 /**
  * Obtains int field values from {@link org.apache.lucene.index.LeafReader#getNumericDocValues} and
@@ -106,28 +104,15 @@ public class EnumFieldSource extends FieldCacheSource {
     return new IntDocValues(this) {
       int lastDocID;
 
-      private int getValueForDoc(int doc) throws IOException {
-        if (doc < lastDocID) {
-          throw new AssertionError(
-              "docs were sent out-of-order: lastDocID=" + lastDocID + " vs doc=" + doc);
-        }
-        lastDocID = doc;
-        int curDocID = arr.docID();
-        if (doc > curDocID) {
-          curDocID = arr.advance(doc);
-        }
-        if (doc == curDocID) {
+      @Override
+      public int intVal(int doc) throws IOException {
+        if (exists(doc)) {
           return (int) arr.longValue();
         } else {
           return 0;
         }
       }
 
-      @Override
-      public int intVal(int doc) throws IOException {
-        return getValueForDoc(doc);
-      }
-
       @Override
       public String strVal(int doc) throws IOException {
         Integer intValue = intVal(doc);
@@ -136,8 +121,16 @@ public class EnumFieldSource extends FieldCacheSource {
 
       @Override
       public boolean exists(int doc) throws IOException {
-        getValueForDoc(doc);
-        return arr.docID() == doc;
+        if (doc < lastDocID) {
+          throw new IllegalArgumentException(
+              "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
+        }
+        lastDocID = doc;
+        int curDocID = arr.docID();
+        if (doc > curDocID) {
+          curDocID = arr.advance(doc);
+        }
+        return doc == curDocID;
       }
 
       @Override
@@ -177,24 +170,6 @@ public class EnumFieldSource extends FieldCacheSource {
           }
         };
       }
-
-      @Override
-      public ValueFiller getValueFiller() {
-        return new ValueFiller() {
-          private final MutableValueInt mval = new MutableValueInt();
-
-          @Override
-          public MutableValue getValue() {
-            return mval;
-          }
-
-          @Override
-          public void fillValue(int doc) throws IOException {
-            mval.value = intVal(doc);
-            mval.exists = arr.docID() == doc;
-          }
-        };
-      }
     };
   }
 
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/FloatFieldSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/FloatFieldSource.java
index b9bd4be31b0..8c6123ed779 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/FloatFieldSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/FloatFieldSource.java
@@ -25,8 +25,6 @@ import org.apache.lucene.queries.function.FunctionValues;
 import org.apache.lucene.queries.function.docvalues.FloatDocValues;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.SortField.Type;
-import org.apache.lucene.util.mutable.MutableValue;
-import org.apache.lucene.util.mutable.MutableValueFloat;
 
 /**
  * Obtains float field values from {@link org.apache.lucene.index.LeafReader#getNumericDocValues}
@@ -57,7 +55,17 @@ public class FloatFieldSource extends FieldCacheSource {
     return new FloatDocValues(this) {
       int lastDocID;
 
-      private float getValueForDoc(int doc) throws IOException {
+      @Override
+      public float floatVal(int doc) throws IOException {
+        if (exists(doc)) {
+          return Float.intBitsToFloat((int) arr.longValue());
+        } else {
+          return 0f;
+        }
+      }
+
+      @Override
+      public boolean exists(int doc) throws IOException {
         if (doc < lastDocID) {
           throw new IllegalArgumentException(
               "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
@@ -67,40 +75,7 @@ public class FloatFieldSource extends FieldCacheSource {
         if (doc > curDocID) {
           curDocID = arr.advance(doc);
         }
-        if (doc == curDocID) {
-          return Float.intBitsToFloat((int) arr.longValue());
-        } else {
-          return 0f;
-        }
-      }
-
-      @Override
-      public float floatVal(int doc) throws IOException {
-        return getValueForDoc(doc);
-      }
-
-      @Override
-      public boolean exists(int doc) throws IOException {
-        getValueForDoc(doc);
-        return arr.docID() == doc;
-      }
-
-      @Override
-      public ValueFiller getValueFiller() {
-        return new ValueFiller() {
-          private final MutableValueFloat mval = new MutableValueFloat();
-
-          @Override
-          public MutableValue getValue() {
-            return mval;
-          }
-
-          @Override
-          public void fillValue(int doc) throws IOException {
-            mval.value = floatVal(doc);
-            mval.exists = arr.docID() == doc;
-          }
-        };
+        return doc == curDocID;
       }
     };
   }
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/IntFieldSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/IntFieldSource.java
index da1c5815968..9f22323fda8 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/IntFieldSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/IntFieldSource.java
@@ -25,8 +25,6 @@ import org.apache.lucene.queries.function.FunctionValues;
 import org.apache.lucene.queries.function.docvalues.IntDocValues;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.SortField.Type;
-import org.apache.lucene.util.mutable.MutableValue;
-import org.apache.lucene.util.mutable.MutableValueInt;
 
 /**
  * Obtains int field values from {@link org.apache.lucene.index.LeafReader#getNumericDocValues} and
@@ -57,28 +55,15 @@ public class IntFieldSource extends FieldCacheSource {
     return new IntDocValues(this) {
       int lastDocID;
 
-      private int getValueForDoc(int doc) throws IOException {
-        if (doc < lastDocID) {
-          throw new IllegalArgumentException(
-              "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
-        }
-        lastDocID = doc;
-        int curDocID = arr.docID();
-        if (doc > curDocID) {
-          curDocID = arr.advance(doc);
-        }
-        if (doc == curDocID) {
+      @Override
+      public int intVal(int doc) throws IOException {
+        if (exists(doc)) {
           return (int) arr.longValue();
         } else {
           return 0;
         }
       }
 
-      @Override
-      public int intVal(int doc) throws IOException {
-        return getValueForDoc(doc);
-      }
-
       @Override
       public String strVal(int doc) throws IOException {
         return Integer.toString(intVal(doc));
@@ -86,26 +71,16 @@ public class IntFieldSource extends FieldCacheSource {
 
       @Override
       public boolean exists(int doc) throws IOException {
-        getValueForDoc(doc);
-        return arr.docID() == doc;
-      }
-
-      @Override
-      public ValueFiller getValueFiller() {
-        return new ValueFiller() {
-          private final MutableValueInt mval = new MutableValueInt();
-
-          @Override
-          public MutableValue getValue() {
-            return mval;
-          }
-
-          @Override
-          public void fillValue(int doc) throws IOException {
-            mval.value = getValueForDoc(doc);
-            mval.exists = arr.docID() == doc;
-          }
-        };
+        if (doc < lastDocID) {
+          throw new IllegalArgumentException(
+              "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
+        }
+        lastDocID = doc;
+        int curDocID = arr.docID();
+        if (doc > curDocID) {
+          curDocID = arr.advance(doc);
+        }
+        return doc == curDocID;
       }
     };
   }
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/LongFieldSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/LongFieldSource.java
index 09bf3f334b4..2d8ad788ffb 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/LongFieldSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/valuesource/LongFieldSource.java
@@ -25,8 +25,6 @@ import org.apache.lucene.queries.function.FunctionValues;
 import org.apache.lucene.queries.function.docvalues.LongDocValues;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.SortField.Type;
-import org.apache.lucene.util.mutable.MutableValue;
-import org.apache.lucene.util.mutable.MutableValueLong;
 
 /**
  * Obtains long field values from {@link org.apache.lucene.index.LeafReader#getNumericDocValues} and
@@ -69,7 +67,17 @@ public class LongFieldSource extends FieldCacheSource {
     return new LongDocValues(this) {
       int lastDocID;
 
-      private long getValueForDoc(int doc) throws IOException {
+      @Override
+      public long longVal(int doc) throws IOException {
+        if (exists(doc)) {
+          return arr.longValue();
+        } else {
+          return 0;
+        }
+      }
+
+      @Override
+      public boolean exists(int doc) throws IOException {
         if (doc < lastDocID) {
           throw new IllegalArgumentException(
               "docs were sent out-of-order: lastDocID=" + lastDocID + " vs docID=" + doc);
@@ -79,28 +87,13 @@ public class LongFieldSource extends FieldCacheSource {
         if (doc > curDocID) {
           curDocID = arr.advance(doc);
         }
-        if (doc == curDocID) {
-          return arr.longValue();
-        } else {
-          return 0;
-        }
-      }
-
-      @Override
-      public long longVal(int doc) throws IOException {
-        return getValueForDoc(doc);
-      }
-
-      @Override
-      public boolean exists(int doc) throws IOException {
-        getValueForDoc(doc);
-        return arr.docID() == doc;
+        return doc == curDocID;
       }
 
       @Override
       public Object objectVal(int doc) throws IOException {
-        long value = getValueForDoc(doc);
-        if (arr.docID() == doc) {
+        if (exists(doc)) {
+          long value = longVal(doc);
           return longToObject(value);
         } else {
           return null;
@@ -109,8 +102,8 @@ public class LongFieldSource extends FieldCacheSource {
 
       @Override
       public String strVal(int doc) throws IOException {
-        long value = getValueForDoc(doc);
-        if (arr.docID() == doc) {
+        if (exists(doc)) {
+          long value = longVal(doc);
           return longToString(value);
         } else {
           return null;
@@ -121,24 +114,6 @@ public class LongFieldSource extends FieldCacheSource {
       protected long externalToLong(String extVal) {
         return LongFieldSource.this.externalToLong(extVal);
       }
-
-      @Override
-      public ValueFiller getValueFiller() {
-        return new ValueFiller() {
-          private final MutableValueLong mval = newMutableValueLong();
-
-          @Override
-          public MutableValue getValue() {
-            return mval;
-          }
-
-          @Override
-          public void fillValue(int doc) throws IOException {
-            mval.value = getValueForDoc(doc);
-            mval.exists = arr.docID() == doc;
-          }
-        };
-      }
     };
   }
 
@@ -147,10 +122,6 @@ public class LongFieldSource extends FieldCacheSource {
     return DocValues.getNumeric(readerContext.reader(), field);
   }
 
-  protected MutableValueLong newMutableValueLong() {
-    return new MutableValueLong();
-  }
-
   @Override
   public boolean equals(Object o) {
     if (o.getClass() != this.getClass()) return false;