[lucene] branch branch_9x updated: Defer #advanceExact on expression dependencies until their values are needed (#12560)

[gs...@apache.org]

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

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


The following commit(s) were added to refs/heads/branch_9x by this push:
     new af978fb27dd Defer #advanceExact on expression dependencies until their values are needed (#12560)
af978fb27dd is described below

commit af978fb27ddfc1710d8f8d2872e05cf5a2112172
Author: Greg Miller <gs...@gmail.com>
AuthorDate: Fri Sep 15 16:26:14 2023 -0700

    Defer #advanceExact on expression dependencies until their values are needed (#12560)
---
 lucene/CHANGES.txt                                 |  3 +
 .../lucene/expressions/ExpressionValueSource.java  | 22 +++++--
 .../expressions/TestExpressionValueSource.java     | 71 ++++++++++++++++++++++
 3 files changed, 92 insertions(+), 4 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index a637a515b3c..8f5a89f119c 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -79,6 +79,9 @@ Optimizations
 
 * GITHUB#12490: Faster computation of top-k hits on boolean queries. (Adrien Grand)
 
+* GITHUB#12560: ExpressionValueSource defers #advanceExact on dependencies until their values are needed, avoiding
+  unnecessary advancing on values that are never evaluated (e.g., because of ternary expressions). (Greg Miller)
+
 Changes in runtime behavior
 ---------------------
 
diff --git a/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java b/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java
index 376323c860c..8208d134a7c 100644
--- a/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java
+++ b/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java
@@ -90,16 +90,30 @@ class ExpressionValueSource extends DoubleValuesSource {
   static DoubleValues zeroWhenUnpositioned(DoubleValues in) {
     return new DoubleValues() {
 
-      boolean positioned = false;
+      int currentDoc = -1;
+      double value;
+      boolean computed = false;
 
       @Override
       public double doubleValue() throws IOException {
-        return positioned ? in.doubleValue() : 0;
+        if (computed == false) {
+          value = in.advanceExact(currentDoc) ? in.doubleValue() : 0;
+          computed = true;
+        }
+        return value;
       }
 
       @Override
-      public boolean advanceExact(int doc) throws IOException {
-        return positioned = in.advanceExact(doc);
+      public boolean advanceExact(int doc) {
+        // This implementation wraps all expression arguments, so we lazily advance it in case the
+        // value is never needed by the expression for a given doc (e.g., ternary branch or
+        // condition short-circuit):
+        if (currentDoc == doc) {
+          return true;
+        }
+        currentDoc = doc;
+        computed = false;
+        return true;
       }
     };
   }
diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionValueSource.java b/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionValueSource.java
index c1ad8c31ab6..87f2e674406 100644
--- a/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionValueSource.java
+++ b/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionValueSource.java
@@ -150,6 +150,30 @@ public class TestExpressionValueSource extends LuceneTestCase {
     assertEquals(fib(n), (int) values.doubleValue());
   }
 
+  public void testLazyDependencies() throws Exception {
+    SimpleBindings bindings = new SimpleBindings();
+    bindings.add("f0", DoubleValuesSource.constant(1));
+    bindings.add("f1", DoubleValuesSource.constant(42));
+    bindings.add("f2", new NoAdvanceDoubleValuesSource());
+
+    // f2 should never be evaluated:
+    Expression expression = JavascriptCompiler.compile("f0 == 1 ? f1 : f2");
+    DoubleValuesSource dvs = expression.getDoubleValuesSource(bindings);
+    DoubleValues dv = dvs.getValues(reader.leaves().get(0), null);
+    dv.advanceExact(0);
+    double value = dv.doubleValue();
+    assertEquals(42, value, 0);
+
+    // one more example to show that we will also correctly short-circuit a condition (f2 should
+    // not be advanced or evaluated):
+    expression = JavascriptCompiler.compile("(1 == 1 || f2) ? f1 : 0");
+    dvs = expression.getDoubleValuesSource(bindings);
+    dv = dvs.getValues(reader.leaves().get(0), null);
+    dv.advanceExact(0);
+    value = dv.doubleValue();
+    assertEquals(42, value, 0);
+  }
+
   private int fib(int n) {
     if (n == 0) {
       return 0;
@@ -215,4 +239,51 @@ public class TestExpressionValueSource extends LuceneTestCase {
       }
     };
   }
+
+  private static class NoAdvanceDoubleValuesSource extends DoubleValuesSource {
+    @Override
+    public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
+      return new DoubleValues() {
+        @Override
+        public double doubleValue() throws IOException {
+          throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean advanceExact(int doc) throws IOException {
+          throw new UnsupportedOperationException();
+        }
+      };
+    }
+
+    @Override
+    public boolean needsScores() {
+      return false;
+    }
+
+    @Override
+    public DoubleValuesSource rewrite(IndexSearcher reader) throws IOException {
+      return this;
+    }
+
+    @Override
+    public int hashCode() {
+      return 0;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      return false;
+    }
+
+    @Override
+    public String toString() {
+      return null;
+    }
+
+    @Override
+    public boolean isCacheable(LeafReaderContext ctx) {
+      return false;
+    }
+  }
 }