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:54:29 UTC
[lucene-solr] branch branch_8x updated: LUCENE-10036: Add factory
method to ScoreCachingWrappingScorer that ensures unnecessary wrapping
doesn't occur (#2534)
This is an automated email from the ASF dual-hosted git repository.
gsmiller pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new f444a69 LUCENE-10036: Add factory method to ScoreCachingWrappingScorer that ensures unnecessary wrapping doesn't occur (#2534)
f444a69 is described below
commit f444a69f703fdd4b9617251411b09a648f955732
Author: Greg Miller <gs...@gmail.com>
AuthorDate: Tue Jul 27 07:54:13 2021 -0700
LUCENE-10036: Add factory method to ScoreCachingWrappingScorer that ensures unnecessary wrapping doesn't occur (#2534)
---
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 | 19 ++++++++++++++-
.../search/TestScoreCachingWrappingScorer.java | 27 ++++++++++++++++++++--
6 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 1282ad2..ff7c3db 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -15,6 +15,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 be9b11f..b941ccc 100644
--- a/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
+++ b/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
@@ -214,11 +214,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 5cb6db8..c7178fa 100644
--- a/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
+++ b/lucene/core/src/java/org/apache/lucene/search/MultiCollector.java
@@ -155,7 +155,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 def8d65..81b3bdb 100644
--- a/lucene/core/src/java/org/apache/lucene/search/PositiveScoresOnlyCollector.java
+++ b/lucene/core/src/java/org/apache/lucene/search/PositiveScoresOnlyCollector.java
@@ -41,7 +41,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 f68f25d..6812427 100644
--- a/lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java
+++ b/lucene/core/src/java/org/apache/lucene/search/ScoreCachingWrappingScorer.java
@@ -38,7 +38,24 @@ public final class ScoreCachingWrappingScorer extends Scorable {
private float curScore;
private final Scorable in;
- /** Creates a new instance by wrapping the given scorer. */
+ /**
+ * 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.
+ * @deprecated Use {@link ScoreCachingWrappingScorer#wrap(Scorable)} instead
+ */
+ @Deprecated
public 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 c3da803..db5bc5c 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestScoreCachingWrappingScorer.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestScoreCachingWrappingScorer.java
@@ -96,7 +96,7 @@ public class TestScoreCachingWrappingScorer extends LuceneTestCase {
}
@Override public void setScorer(Scorable scorer) {
- this.scorer = new ScoreCachingWrappingScorer(scorer);
+ this.scorer = ScoreCachingWrappingScorer.wrap(scorer);
}
@Override
@@ -134,5 +134,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);
+ }
}