Leverage sorted dim when visiting BKD leaf. by vsop-479 · Pull Request #12528 · apache/lucene

vsop-479 · 2023-08-30T07:25:01Z

Since values are sorted on the sorted dim in BKD leaf, early terminate the visit when current packed value greater than upper point in sorted dimension, may get a better performance.

Here are some performance data:

1 dim point

Visit method for low cardinality points:

baseline ns	candidate ns	speedup
52017	41400	20.4%

Whole search for low cardinality points:

baseline ns	candidate ns	speedup
242058	228225	5.7%

Whole search for high cardinality points:

baseline ns	candidate ns	speedup
984615	868358	11.8%

2 dim point

Whole search for low cardinality points:

baseline ns	candidate ns	speedup
1748439	1656944	5.2%

Whole search for high cardinality points:

baseline ns	candidate ns	speedup
1655624	1446875	12.6%

vsop-479 · 2023-09-20T08:23:14Z

@jpountz Please take a look when you get a chance!

vsop-479 · 2023-09-22T07:24:01Z

@iverase I replaced int values with static variables. Please take a look.
Actually, i used enum to define the match states in pre version. but it downgraded the performance a little.
Static variables is good, but do you think it is ok to use enum to make code graceful, even through there is little performance downgrade?

iverase · 2023-09-22T08:01:09Z

+    public static final int HIGH_IN_SORTED_DIM = 2;
+    /** Packed value is too high in NON-SORTED dimension */
+    public static final int HIGH_IN_NON_SORTED_DIM = 3;
+  }


My main concern here is that the concept of SORTED dimension does not exist in the PointValues API. If you have a look to the javadocs when visiting a leaf node:

/** * Called for all documents in a leaf cell that crosses the query. The consumer should * scrutinize the packedValue to decide whether to accept it. In the 1D case, values are visited * in increasing order, and in the case of ties, in increasing docID order. */

It only constraints the 1D case but in higher dimensions there is no constraint how data is visited. The concept of SORTED dimension sounds to me an implementation detail that should not be leaked to the public API.

iverase

I have concerns with this change as I have the impression that we are leaking implementation details into the public API which is not a good idea in general. Maybe other folks have a different opinion but I don't think the leaves sorted dimensions should be part of the public API, or at least we need to make it explicit before this change.

…ator, byte[]) to make other queries use DocIdSetIterator.

vsop-479 · 2023-09-25T07:01:30Z

@iverase Does it make sense to you if MatchState defined in other class, such as BKDReader or IntersectVisitor, and only leave the sorted dimension in IntersectVisitor' visit method as a parameter?
I just implemented visitWithState in PointRangeQuery, and there is some speed up. I think other queries, such as LatLonPointDistanceQuery, which use a similar IntersectVisitor, can get a better performance too.

iverase · 2023-09-25T11:02:44Z

My recommendation is to use the following method as you are just trying to flag if the visit method needs to keep processing points.

  /** Similar to {@link IntersectVisitor#visit(int, byte[])} but data is visited in 
   * increasing order on the {@sortedDim}, and in the case of ties, in increasing docID order. 
   * Implementers can stop processing points on the leaf by returning false when for example the 
   * sorted dimension value is too high to be matched by the query. 
   * 
   * @return true if the visitor should continue visiting points on this leaf, otherwise false. 
   * */

 default boolean visitWithSortedDim(int docID, byte[] packedValue, int sortedDim) throws IOException { 
       visit(docID, packedValue); 
        return true; 
   }

I would remove the "inverse" case as the inverse visitors are implementation details and IMHO should not be part of the API.

just implemented visitWithState in PointRangeQuery, and there is some speed up

We should run the performance test in the [lucene benchmarks](lucene benchmarks) to check if there are slowdowns due to the extra check for each visited point.

vsop-479 · 2023-09-26T01:44:07Z

@iverase Thanks for your recommendation, it may makes code more clear.
I will try to implement it, and run the performance test.

vsop-479 · 2023-10-10T09:30:07Z

@iverase Here is a performance test from luceneutil.

box-points	baseline	candidate	Diff
BEST M hits/sec	101.09	103.64	2.5%
BEST QPS	102.87	105.46	2.5%

I only implemented visitWithSortedDim in PointRangeQuery (without "inverse" case) temporarily, so I just ran the box's points test.

run the performance test in the [lucene benchmarks]

Do you mean the off the shelf benchmark in lucene self?

iverase · 2023-10-10T10:33:27Z

Do you mean the off the shelf benchmark in lucene self?

One case I am interested in is the geo benchmarks. It is not clear to me if some of those queries (e.g polygon query) can effectively take advantage of this change so it would be good to know that they are not slowed down.

vsop-479 · 2023-10-11T07:29:34Z

@iverase
Here is a performance data of geo cases. There are some slowdown due to the extra check.

query	metric	baseline	candidate	Diff
poly-10-geo3d	BEST M hits/sec	68.13	67.99	-0.2%
--	BEST QPS	43.07	42.99	-0.1%
box-geo3d	BEST M hits/sec	73.63	72.86	-0.1%
--	BEST QPS	74.92	74.14	-0.1%

I want to add a enhanced IntersectVisitor type or add a flag to IntersectVisitor, for queries which have already implemented visitWithSortedDim. And leave queries which haven't or couldn't implement visitWithSortedDim, use the original visit method to avoid the unnecessary checks.
Does it make sense to you?

vsop-479 · 2023-11-07T09:47:32Z

@iverase I implemented visitWithSortedDim in LatLonPointDistanceQuery. Please take a look when you get a chance!

BTW, The contents of getIntersectVisitor and getInverseIntersectVisitor in LatLonPointDistanceQuery and PointRangeQuery are identical. Should we move them to a DefaultIntersectVisitor and make match/relate abstract in it. I mean in a new PR without this change.

github-actions · 2024-01-08T12:23:45Z

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

vsop-479 · 2025-05-12T12:47:01Z

I would remove the "inverse" case as the inverse visitors are implementation details and IMHO should not be part of the API.

FWIW, I implemented "inverse" case with adding a VisitState.

vsop-479 · 2025-05-12T12:48:11Z

I will try to measure the performance.

vsop-479 · 2025-05-13T12:53:36Z

Performance with "inverse" visitor implemented:

box-points	baseline	candidate	Diff
BEST M hits/sec	500.14	513.51	2.6%
BEST QPS	508.92	522.53	2.6%

vsop-479 · 2025-05-15T03:40:43Z

@jpountz Please take a look when you get a chance.

vsop-479 · 2025-05-16T03:13:13Z

Performance details:

baseline:

ITER 0: 301.73 M hits/sec, 307.03 QPS (0.73 sec for 225 queries), totHits=221118844
ITER 1: 483.36 M hits/sec, 491.84 QPS (0.46 sec for 225 queries), totHits=221118844
ITER 2: 479.59 M hits/sec, 488.01 QPS (0.46 sec for 225 queries), totHits=221118844
ITER 3: 496.58 M hits/sec, 505.29 QPS (0.45 sec for 225 queries), totHits=221118844
ITER 4: 496.42 M hits/sec, 505.13 QPS (0.45 sec for 225 queries), totHits=221118844
ITER 5: 499.83 M hits/sec, 508.60 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 6: 495.70 M hits/sec, 504.40 QPS (0.45 sec for 225 queries), totHits=221118844
ITER 7: 498.27 M hits/sec, 507.01 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 8: 499.43 M hits/sec, 508.20 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 9: 499.35 M hits/sec, 508.12 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 10: 499.80 M hits/sec, 508.58 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 11: 499.80 M hits/sec, 508.57 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 12: 497.82 M hits/sec, 506.56 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 13: 499.86 M hits/sec, 508.63 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 14: 497.54 M hits/sec, 506.28 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 15: 499.22 M hits/sec, 507.98 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 16: 499.23 M hits/sec, 508.00 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 17: 498.53 M hits/sec, 507.28 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 18: 500.15 M hits/sec, 508.92 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 19: 497.59 M hits/sec, 506.33 QPS (0.44 sec for 225 queries), totHits=221118844
BEST M hits/sec: 500.1453034653827
BEST QPS: 508.9240303721519

candidate:

ITER 0: 327.93 M hits/sec, 333.69 QPS (0.67 sec for 225 queries), totHits=221118844
ITER 1: 490.06 M hits/sec, 498.66 QPS (0.45 sec for 225 queries), totHits=221118844
ITER 2: 504.86 M hits/sec, 513.72 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 3: 504.06 M hits/sec, 512.91 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 4: 513.52 M hits/sec, 522.53 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 5: 503.72 M hits/sec, 512.56 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 6: 503.84 M hits/sec, 512.68 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 7: 505.70 M hits/sec, 514.58 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 8: 509.43 M hits/sec, 518.37 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 9: 509.74 M hits/sec, 518.69 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 10: 506.50 M hits/sec, 515.39 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 11: 510.44 M hits/sec, 519.40 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 12: 506.21 M hits/sec, 515.10 QPS (0.44 sec for 225 queries), totHits=221118844
ITER 13: 508.55 M hits/sec, 517.48 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 14: 510.95 M hits/sec, 519.92 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 15: 511.24 M hits/sec, 520.21 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 16: 508.64 M hits/sec, 517.57 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 17: 512.81 M hits/sec, 521.81 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 18: 511.31 M hits/sec, 520.29 QPS (0.43 sec for 225 queries), totHits=221118844
ITER 19: 510.67 M hits/sec, 519.64 QPS (0.43 sec for 225 queries), totHits=221118844
BEST M hits/sec: 513.5193512565019
BEST QPS: 522.5328241708469

github-actions · 2025-05-31T00:26:44Z

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

github-actions · 2025-10-17T00:27:15Z

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

Copilot

Pull request overview

This PR optimizes BKD leaf visits by leveraging the fact that leaf values are sorted on a “sorted dimension”, enabling early termination (or bulk matching of remaining docs for inverse queries) when the current value is beyond the query’s upper bound on that dimension.

Changes:

Add VisitState / MatchState and new IntersectVisitor#visitWithSortedDim(...) hooks to support early-termination semantics.
Update BKD leaf decoding to call visitWithSortedDim(...) for both sparse (low-cardinality) and compressed (high-cardinality) leaves.
Extend point query implementations and add new tests to cover sparse/compressed scenarios.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
lucene/test-framework/src/java/org/apache/lucene/tests/index/AssertingLeafReader.java	Adds assertions for new `visitWithSortedDim` APIs (but currently weakens `docBudget` validation for bulk paths).
lucene/core/src/test/org/apache/lucene/search/TestPointQueries.java	Adds new tests covering sparse/compressed leaf behavior for the new visitor mode.
lucene/core/src/java/org/apache/lucene/util/bkd/BKDWriter.java	Writes additional metadata for low-cardinality leaves to support sorted-dim visiting.
lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java	Reads new metadata and performs early-termination-aware visiting while decoding leaf values.
lucene/core/src/java/org/apache/lucene/search/PointRangeQuery.java	Implements early termination/match-remaining behavior for point range queries.
lucene/core/src/java/org/apache/lucene/index/PointValues.java	Introduces new enums and default visitor methods to support sorted-dim visiting.
lucene/core/src/java/org/apache/lucene/document/LatLonPointDistanceQuery.java	Implements early termination/match-remaining behavior for distance queries based on sorted dim.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

        out.writeByte((byte) -2);
+        out.writeByte((byte) sortedDim);


+      if (sortedDim == -2) {
+        sortedDim = in.readByte();


+          break;
+        } else if (visitState == VisitState.MATCH_REMAINING) {
+          scratchIterator.reset(i, count - i);
+          visitor.visit(scratchIterator);
+          break;
+        }
        i += length;
      }


+          break;
+        }
+        i += runLen;
+      }


+      // TODO: docBudget -= iterator.length.
+      //      assert --docBudget >= 0 : "called add() more times than the last call to grow()
+      // reserved";


+  public void testCompressedWithSoredDimIntersectVisitor() throws Exception {
+    Directory dir = newDirectory();
+    IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(new MockAnalyzer(random())));
+


+  public void testCompressedWithSoredDimInverseIntersectVisitor() throws Exception {
+    Directory dir = newDirectory();
+    IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(new MockAnalyzer(random())));
+


+    MATCH_REMAINING
+  };
+
+  /** Math states for current value. */


+    /**
+     * Similar to {@link IntersectVisitor#visit(int, byte[])} but data is visited in increasing
+     * order on the sortedDim, and in the case of ties, in increasing docID order. Implementers can
+     * stop processing points on the leaf by returning false when for example the sorted dimension
+     * value is too high to be matched by the query.
+     *
+     * @return VisitState.CONTINUE if the visitor should continue visiting points on this leaf,
+     *     otherwise false.
+     */
+    default VisitState visitWithSortedDim(int docID, byte[] packedValue, int sortedDim)
+        throws IOException {
+      visit(docID, packedValue);
+      return VisitState.CONTINUE;
+    }


+          //        // cross
+          //          if (lon > maxLon && lon < minLon2) {}
+          //        // disable
+          //          if (lon > maxLon || lon < minLon) {}


github-actions · 2026-05-07T00:52:54Z

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

vsop-479 added 3 commits August 30, 2023 14:42

Early terminate visit bkd leaf for one dim point.

5b97476

Tidy.

4cbb76e

Early terminate for high cardinality BKD leaf.

b1ddd1a

vsop-479 changed the title ~~Early terminate visit low cardinality BKD leaf when current value greater than upper point for one dim point.~~ Early terminate visit BKD leaf when current value greater than upper point for one dim point. Sep 1, 2023

vsop-479 added 2 commits September 2, 2023 08:52

Early terminate for inverse visitor.

82550bf

Early terminate for N dims point.

14a8bee

vsop-479 changed the title ~~Early terminate visit BKD leaf when current value greater than upper point for one dim point.~~ Early terminate visit BKD leaf when current value greater than upper point in sorted dim. Sep 5, 2023

iverase reviewed Sep 21, 2023

View reviewed changes

Comment thread lucene/core/src/java/org/apache/lucene/util/bkd/BKDWriter.java

iverase reviewed Sep 21, 2023

View reviewed changes

Comment thread lucene/core/src/java/org/apache/lucene/index/PointValues.java Outdated

Use static variables instead of int values.

8304c15

iverase reviewed Sep 22, 2023

View reviewed changes

Comment thread lucene/core/src/java/org/apache/lucene/index/PointValues.java Outdated

iverase reviewed Sep 22, 2023

View reviewed changes

vsop-479 added 3 commits September 22, 2023 16:41

Use static variables instead of remain int values.

b465046

Invoke visit(DocIdSetIterator, byte[]) in visitWithState(DocIdSetIter…

d713d96

…ator, byte[]) to make other queries use DocIdSetIterator.

Modify some comments.

9c24eeb

Visit with sorted dim.

b85a9b2

Implement visitWithSortedDim in LatLonPointDistanceQuery

5584de2

Tidy.

2066e4f

github-actions Bot added the Stale label Jan 8, 2024

github-actions Bot added the module:test-framework label May 6, 2025

update.

bf32dd1

vsop-479 requested a review from iverase May 6, 2025 09:33

github-actions Bot removed the Stale label May 7, 2025

vsop-479 added 7 commits May 9, 2025 20:04

Add VisitState to implement inverse visit with sorted dim.

8868f86

Iter.

de65346

Leave compared doc/iterator to remaining.

b3f8c59

Change MatchState to enum.

1e6e690

Revert PointRangeQuery to resolve conflicts.

4f69bea

Merge branch 'main' into early_terminate_visit_bkd_leaf

107af45

Implement visit with sorted dim.

a148a69

vsop-479 changed the title ~~Early terminate visit BKD leaf when current value greater than upper point in sorted dim.~~ Leverage sorted dim when visiting bkd leaf. May 14, 2025

vsop-479 changed the title ~~Leverage sorted dim when visiting bkd leaf.~~ Leverage sorted dim when visiting BKD leaf. May 14, 2025

github-actions Bot added the Stale label May 31, 2025

github-actions Bot removed the Stale label Oct 1, 2025

github-actions Bot added the Stale label Oct 17, 2025

vsop-479 requested a review from Copilot April 22, 2026 08:30

Copilot started reviewing on behalf of vsop-479 April 22, 2026 08:30 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

github-actions Bot removed the Stale label Apr 23, 2026

github-actions Bot added the Stale label May 7, 2026

Merge branch 'main' into early_terminate_visit_bkd_leaf

f1f8c3f

Uh oh!

Conversation

vsop-479 commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1 dim point

2 dim point

Uh oh!

vsop-479 commented Sep 20, 2023

Uh oh!

Uh oh!

Uh oh!

vsop-479 commented Sep 22, 2023

Uh oh!

iverase Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iverase left a comment

Choose a reason for hiding this comment

Uh oh!

vsop-479 commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iverase commented Sep 25, 2023

Uh oh!

vsop-479 commented Sep 26, 2023

Uh oh!

vsop-479 commented Oct 10, 2023

Uh oh!

iverase commented Oct 10, 2023

Uh oh!

vsop-479 commented Oct 11, 2023

Uh oh!

vsop-479 commented Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jan 8, 2024

Uh oh!

vsop-479 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsop-479 commented May 12, 2025

Uh oh!

vsop-479 commented May 13, 2025

Uh oh!

vsop-479 commented May 15, 2025

Uh oh!

vsop-479 commented May 16, 2025

Uh oh!

github-actions Bot commented May 31, 2025

Uh oh!

github-actions Bot commented Oct 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vsop-479 commented Aug 30, 2023 •

edited

Loading

vsop-479 commented Sep 25, 2023 •

edited

Loading

vsop-479 commented Nov 7, 2023 •

edited

Loading

vsop-479 commented May 12, 2025 •

edited

Loading