From 7f50732cc3e7281359c6952016cfc7927f997c63 Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Wed, 9 Oct 2024 12:33:34 +0200 Subject: [PATCH] Avoid performance regression by constructing lazily the PointTree in NumericComparator (#13498) (#13877) --- lucene/CHANGES.txt | 2 ++ .../search/comparators/NumericComparator.java | 15 +++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 4ffd1bb76fe..97dae1e89d8 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -307,6 +307,8 @@ Bug Fixes * GITHUB#12878: Fix the declared Exceptions of Expression#evaluate() to match those of DoubleValues#doubleValue(). (Uwe Schindler) +* GITHUB#13498: Avoid performance regression by constructing lazily the PointTree in NumericComparator, (Ignacio Vera) + Changes in Runtime Behavior --------------------- diff --git a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java index 31022f6bb05..2bd594cc731 100644 --- a/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java +++ b/lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java @@ -95,7 +95,8 @@ public abstract class NumericComparator extends FieldComparato private final LeafReaderContext context; protected final NumericDocValues docValues; private final PointValues pointValues; - private final PointValues.PointTree pointTree; + // lazily constructed to avoid performance overhead when this is not used + private PointValues.PointTree pointTree; // if skipping functionality should be enabled on this segment private final boolean enableSkipping; private final int maxDoc; @@ -139,7 +140,6 @@ public abstract class NumericComparator extends FieldComparato + " expected " + bytesCount); } - this.pointTree = pointValues.getPointTree(); this.enableSkipping = true; // skipping is enabled when points are available this.maxDoc = context.reader().maxDoc(); this.competitiveIterator = DocIdSetIterator.all(maxDoc); @@ -147,7 +147,6 @@ public abstract class NumericComparator extends FieldComparato encodeTop(); } } else { - this.pointTree = null; this.enableSkipping = false; this.maxDoc = 0; } @@ -273,7 +272,8 @@ public abstract class NumericComparator extends FieldComparato final long threshold = iteratorCost >>> 3; - if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo(visitor, pointTree, threshold)) { + if (PointValues.isEstimatedPointCountGreaterThanOrEqualTo( + visitor, getPointTree(), threshold)) { // the new range is not selective enough to be worth materializing, it doesn't reduce number // of docs at least 8x updateSkipInterval(false); @@ -290,6 +290,13 @@ public abstract class NumericComparator extends FieldComparato updateSkipInterval(true); } + private PointValues.PointTree getPointTree() throws IOException { + if (pointTree == null) { + pointTree = pointValues.getPointTree(); + } + return pointTree; + } + private void updateSkipInterval(boolean success) { if (updateCounter > 256) { if (success) {