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 2021/07/27 14:53:43 UTC
[lucene] branch main updated: LUCENE-10036: Add factory method to
ScoreCachingWrappingScorer that ensures unnecessary wrapping doesn't occur
(#226)
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 e44636c LUCENE-10036: Add factory method to ScoreCachingWrappingScorer that ensures unnecessary wrapping doesn't occur (#226)
e44636c is described below
commit e44636c28080055502bd30a42c8b1c12801d6b75
Author: Greg Miller <gs...@gmail.com>
AuthorDate: Tue Jul 27 07:53:36 2021 -0700
LUCENE-10036: Add factory method to ScoreCachingWrappingScorer that ensures unnecessary wrapping doesn't occur (#226)
---
lucene/CHANGES.txt | 3 +++
.../org/apache/lucene/search/FieldComparator.java | 6 +----
.../org/apache/lucene/search/MultiCollector.java | 2 +-
.../lucene/search/PositiveScoresOnlyCollector.java | 2 +-
.../lucene/search/ScoreCachingWrappingScorer.java | 16 ++++++++++++-
.../search/TestScoreCachingWrappingScorer.java | 26 +++++++++++++++++++++-
.../apache/lucene/facet/DrillSidewaysScorer.java | 2 +-
7 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index e0c8cbb..b2dbfba 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -357,6 +357,9 @@ API Changes
Users can now access the count of an ordinal directly without constructing an extra FacetLabel.
Also use variable length arguments for the getOrdinal call in TaxonomyReader. (Gautam Worah)
+* LUCENE-10036: Replaced the ScoreCachingWrappingScorer ctor with a static factory method that
+ ensures unnecessary wrapping doesn't occur. (Greg Miller)
+
New Features
---------------------
(No changes)
diff --git a/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java b/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
index 90db5ac..e92436c 100644
--- a/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
+++ b/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
@@ -187,11 +187,7 @@ public abstract class FieldComparator<T> {
// wrap with a ScoreCachingWrappingScorer so that successive calls to
// score() will not incur score computation over and
// over again.
- if (!(scorer instanceof ScoreCachingWrappingScorer)) {
- this.scorer = new ScoreCachingWrappingScorer(scorer);
- } else {
- this.scorer = scorer;
- }
+ this.scorer = ScoreCachingWrappingScorer.wrap(scorer);
}
@Override
diff --git a/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java b/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
index 7e54199..273106c 100644
--- a/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
+++ b/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
@@ -158,7 +158,7 @@ public class MultiCollector implements Collector {
@Override
public void setScorer(Scorable scorer) throws IOException {
if (cacheScores) {
- scorer = new ScoreCachingWrappingScorer(scorer);
+ scorer = ScoreCachingWrappingScorer.wrap(scorer);
}
if (skipNonCompetitiveScores) {
for (int i = 0; i < collectors.length; ++i) {
diff --git a/lucene/core/src/java/org/apache/lucene/search/PositiveScoresOnlyCollector.java b/lucene/core/src/java/org/apache/lucene/search/PositiveScoresOnlyCollector.java
index b8bf307..7dd0b45 100644
--- a/lucene/core/src/java/org/apache/lucene/search/PositiveScoresOnlyCollector.java
+++ b/lucene/core/src/java/org/apache/lucene/search/PositiveScoresOnlyCollector.java
@@ -37,7 +37,7 @@ public class PositiveScoresOnlyCollector extends FilterCollector {
@Override
public void setScorer(Scorable scorer) throws IOException {
- this.scorer = new ScoreCachingWrappingScorer(scorer);
+ this.scorer = ScoreCachingWrappingScorer.wrap(scorer);
in.setScorer(this.scorer);
}
diff --git a/lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java b/lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java
index a8820b2..ebdc826 100644
--- a/lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java
+++ b/lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java
@@ -35,8 +35,22 @@ public final class ScoreCachingWrappingScorer extends Scorable {
private float curScore;
private final Scorable in;
+ /**
+ * Wraps the provided {@link Scorable} unless it's already an instance of {@code
+ * ScoreCachingWrappingScorer}, in which case it will just return the provided instance.
+ *
+ * @param scorer Underlying {@code Scorable} to wrap
+ * @return Instance of {@code ScoreCachingWrappingScorer} wrapping the underlying {@code scorer}
+ */
+ public static Scorable wrap(Scorable scorer) {
+ if (scorer instanceof ScoreCachingWrappingScorer) {
+ return scorer;
+ }
+ return new ScoreCachingWrappingScorer(scorer);
+ }
+
/** Creates a new instance by wrapping the given scorer. */
- public ScoreCachingWrappingScorer(Scorable scorer) {
+ private ScoreCachingWrappingScorer(Scorable scorer) {
this.in = scorer;
}
diff --git a/lucene/core/src/test/org/apache/lucene/search/TestScoreCachingWrappingScorer.java b/lucene/core/src/test/org/apache/lucene/search/TestScoreCachingWrappingScorer.java
index 8ac3f54..b5f1bcf 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestScoreCachingWrappingScorer.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestScoreCachingWrappingScorer.java
@@ -105,7 +105,7 @@ public class TestScoreCachingWrappingScorer extends LuceneTestCase {
@Override
public void setScorer(Scorable scorer) {
- this.scorer = new ScoreCachingWrappingScorer(scorer);
+ this.scorer = ScoreCachingWrappingScorer.wrap(scorer);
}
@Override
@@ -156,4 +156,28 @@ public class TestScoreCachingWrappingScorer extends LuceneTestCase {
ir.close();
directory.close();
}
+
+ public void testNoUnnecessaryWrap() throws Exception {
+ Scorable base =
+ new Scorable() {
+ @Override
+ public float score() throws IOException {
+ return -1;
+ }
+
+ @Override
+ public int docID() {
+ return -1;
+ }
+ };
+
+ // Wrapping the first time should produce a different instance:
+ Scorable wrapped = ScoreCachingWrappingScorer.wrap(base);
+ assertNotEquals(base, wrapped);
+
+ // But if we try to wrap an instance of ScoreCachingWrappingScorer, it shouldn't unnecessarily
+ // wrap again:
+ Scorable doubleWrapped = ScoreCachingWrappingScorer.wrap(wrapped);
+ assertSame(wrapped, doubleWrapped);
+ }
}
diff --git a/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java b/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java
index 8d43462..bf006f3 100644
--- a/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java
+++ b/lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysScorer.java
@@ -152,7 +152,7 @@ class DrillSidewaysScorer extends BulkScorer {
// if (DEBUG) {
// System.out.println(" doQueryFirstScoring");
// }
- setScorer(collector, new ScoreCachingWrappingScorer(baseScorer));
+ setScorer(collector, ScoreCachingWrappingScorer.wrap(baseScorer));
int docID = baseScorer.docID();