You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2025/06/10 19:36:39 UTC
(lucene) branch main updated: Revert "Better vectorize score computations. (#14704)"
This is an automated email from the ASF dual-hosted git repository.
jpountz 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 544d21b0b43 Revert "Better vectorize score computations. (#14704)"
544d21b0b43 is described below
commit 544d21b0b4362d8459b883df5b7818f568167b7a
Author: Adrien Grand <jp...@gmail.com>
AuthorDate: Tue Jun 10 21:36:02 2025 +0200
Revert "Better vectorize score computations. (#14704)"
This reverts commit abe18a4b07bb3240f7cf0e0d22c509f25bfee504.
---
lucene/CHANGES.txt | 3 -
.../java/org/apache/lucene/search/TermScorer.java | 27 +++++-
.../lucene/search/similarities/BM25Similarity.java | 58 +------------
.../lucene/search/similarities/Similarity.java | 25 ------
.../search/similarities/AssertingSimilarity.java | 19 -----
.../similarities/BaseSimilarityTestCase.java | 98 ----------------------
6 files changed, 27 insertions(+), 203 deletions(-)
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index f5c2ee49b5a..1113c7b5e8e 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -150,9 +150,6 @@ Optimizations
* GITHUB#14753: Implement IndexedDISI#docIDRunEnd. (Ge Song)
-* GITHUB#14704: More reliably (auto-)vectorize score computations.
- (Guo Feng, Adrien Grand)
-
Bug Fixes
---------------------
* GITHUB#14654: ValueSource.fromDoubleValuesSource(dvs).getSortField() would throw errors when
diff --git a/lucene/core/src/java/org/apache/lucene/search/TermScorer.java b/lucene/core/src/java/org/apache/lucene/search/TermScorer.java
index d491a1c0633..278f735838e 100644
--- a/lucene/core/src/java/org/apache/lucene/search/TermScorer.java
+++ b/lucene/core/src/java/org/apache/lucene/search/TermScorer.java
@@ -17,12 +17,15 @@
package org.apache.lucene.search;
import java.io.IOException;
+import java.util.Arrays;
import org.apache.lucene.index.ImpactsEnum;
import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.index.PostingsEnum;
import org.apache.lucene.index.SlowImpactsEnum;
import org.apache.lucene.search.similarities.Similarity.SimScorer;
+import org.apache.lucene.util.ArrayUtil;
import org.apache.lucene.util.Bits;
+import org.apache.lucene.util.LongsRef;
/**
* Expert: A <code>Scorer</code> for documents matching a <code>Term</code>.
@@ -36,6 +39,7 @@ public final class TermScorer extends Scorer {
private final NumericDocValues norms;
private final ImpactsDISI impactsDisi;
private final MaxScoreCache maxScoreCache;
+ private long[] normValues = LongsRef.EMPTY_LONGS;
/** Construct a {@link TermScorer} that will iterate all documents. */
public TermScorer(PostingsEnum postingsEnum, SimScorer scorer, NumericDocValues norms) {
@@ -144,6 +148,27 @@ public final class TermScorer extends Scorer {
break;
}
- scorer.score(buffer, norms);
+ int size = buffer.size;
+ if (normValues.length < size) {
+ normValues = new long[ArrayUtil.oversize(size, Long.BYTES)];
+ if (norms == null) {
+ Arrays.fill(normValues, 1L);
+ }
+ }
+ if (norms != null) {
+ for (int i = 0; i < size; ++i) {
+ if (norms.advanceExact(buffer.docs[i])) {
+ normValues[i] = norms.longValue();
+ } else {
+ normValues[i] = 1L;
+ }
+ }
+ }
+
+ for (int i = 0; i < size; ++i) {
+ // Unless SimScorer#score is megamorphic, SimScorer#score should inline and (part of) score
+ // computations should auto-vectorize.
+ buffer.features[i] = scorer.score(buffer.features[i], normValues[i]);
+ }
}
}
diff --git a/lucene/core/src/java/org/apache/lucene/search/similarities/BM25Similarity.java b/lucene/core/src/java/org/apache/lucene/search/similarities/BM25Similarity.java
index f9d75adf6b4..274bb475d6c 100644
--- a/lucene/core/src/java/org/apache/lucene/search/similarities/BM25Similarity.java
+++ b/lucene/core/src/java/org/apache/lucene/search/similarities/BM25Similarity.java
@@ -16,15 +16,11 @@
*/
package org.apache.lucene.search.similarities;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.search.CollectionStatistics;
-import org.apache.lucene.search.DocAndFloatFeatureBuffer;
import org.apache.lucene.search.Explanation;
import org.apache.lucene.search.TermStatistics;
-import org.apache.lucene.util.ArrayUtil;
import org.apache.lucene.util.SmallFloat;
/**
@@ -211,12 +207,6 @@ public class BM25Similarity extends Similarity {
/** weight (idf * boost) */
private final float weight;
- /**
- * Temporary array to store norm inverses and help {@link #score(DocAndFloatFeatureBuffer,
- * NumericDocValues)} auto-vectorize.
- */
- private float[] normInverses = null;
-
BM25Scorer(float boost, float k1, float b, Explanation idf, float avgdl, float[] cache) {
this.boost = boost;
this.idf = idf;
@@ -227,21 +217,6 @@ public class BM25Similarity extends Similarity {
this.weight = boost * idf.getValue().floatValue();
}
- // Don't call it `score` to avoid mistakenly calling score(float, float) instead of score(float,
- // long) or vice-versa.
- private float doScore(float freq, float normInverse) {
- // In order to guarantee monotonicity with both freq and norm without
- // promoting to doubles, we rewrite freq / (freq + norm) to
- // 1 - 1 / (1 + freq * 1/norm).
- // freq * 1/norm is guaranteed to be monotonic for both freq and norm due
- // to the fact that multiplication and division round to the nearest
- // float. And then monotonicity is preserved through composition via
- // x -> 1 + x and x -> 1 - 1/x.
- // Finally we expand weight * (1 - 1 / (1 + freq * 1/norm)) to
- // weight - weight / (1 + freq * 1/norm), which runs slightly faster.
- return weight - weight / (1f + freq * normInverse);
- }
-
@Override
public float score(float freq, long encodedNorm) {
// In order to guarantee monotonicity with both freq and norm without
@@ -254,38 +229,7 @@ public class BM25Similarity extends Similarity {
// Finally we expand weight * (1 - 1 / (1 + freq * 1/norm)) to
// weight - weight / (1 + freq * 1/norm), which runs slightly faster.
float normInverse = cache[((byte) encodedNorm) & 0xFF];
- return doScore(freq, normInverse);
- }
-
- @Override
- public void score(DocAndFloatFeatureBuffer buffer, NumericDocValues norms) throws IOException {
- if (norms == null) {
- float normInverse = cache[1];
- // The below loop should auto-vectorize.
- for (int i = 0; i < buffer.size; ++i) {
- buffer.features[i] = doScore(buffer.features[i], normInverse);
- }
- } else {
- if (normInverses == null || normInverses.length < buffer.size) {
- normInverses = new float[ArrayUtil.oversize(buffer.size, Float.BYTES)];
- }
-
- for (int i = 0; i < buffer.size; ++i) {
- if (norms.advanceExact(buffer.docs[i])) {
- // If norms#longValue gets inlined, the JVM compiler should hopefully detect that a byte
- // is expanded to a long and then casted back to the same original byte, and ignore
- // these operations.
- normInverses[i] = cache[((byte) norms.longValue()) & 0xFF];
- } else {
- normInverses[i] = cache[1];
- }
- }
-
- // The below loop should auto-vectorize
- for (int i = 0; i < buffer.size; ++i) {
- buffer.features[i] = doScore(buffer.features[i], normInverses[i]);
- }
- }
+ return weight - weight / (1f + freq * normInverse);
}
@Override
diff --git a/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java b/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java
index 09fdc73aa52..83582e44e25 100644
--- a/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java
+++ b/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java
@@ -16,15 +16,12 @@
*/
package org.apache.lucene.search.similarities;
-import java.io.IOException;
import java.util.Collections;
import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
import org.apache.lucene.document.NumericDocValuesField;
import org.apache.lucene.index.FieldInvertState;
import org.apache.lucene.index.IndexOptions;
-import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.search.CollectionStatistics;
-import org.apache.lucene.search.DocAndFloatFeatureBuffer;
import org.apache.lucene.search.Explanation;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.TermStatistics;
@@ -211,28 +208,6 @@ public abstract class Similarity {
*/
public abstract float score(float freq, long norm);
- /**
- * Bulk computation of scores. For each entry in the given {@code buffer}, interpreting features
- * as term frequencies, update features to record the score instead.
- *
- * <p><b>NOTE</b>: Doc IDs must be sorted, with no duplicates.
- *
- * <p><b>NOTE</b>: {@code norms} may be null if norms are not indexed.
- *
- * @lucene.internal
- */
- public void score(DocAndFloatFeatureBuffer buffer, NumericDocValues norms) throws IOException {
- for (int i = 0; i < buffer.size; ++i) {
- long norm;
- if (norms == null || norms.advanceExact(buffer.docs[i]) == false) {
- norm = 1L;
- } else {
- norm = norms.longValue();
- }
- buffer.features[i] = score(buffer.features[i], norm);
- }
- }
-
/**
* Explain the score for a single document
*
diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/AssertingSimilarity.java b/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/AssertingSimilarity.java
index fa6c41e600a..b4bf4af31ba 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/AssertingSimilarity.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/AssertingSimilarity.java
@@ -16,11 +16,8 @@
*/
package org.apache.lucene.tests.search.similarities;
-import java.io.IOException;
import org.apache.lucene.index.FieldInvertState;
-import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.search.CollectionStatistics;
-import org.apache.lucene.search.DocAndFloatFeatureBuffer;
import org.apache.lucene.search.Explanation;
import org.apache.lucene.search.TermStatistics;
import org.apache.lucene.search.similarities.Similarity;
@@ -88,22 +85,6 @@ public class AssertingSimilarity extends Similarity {
return score;
}
- @Override
- public void score(DocAndFloatFeatureBuffer buffer, NumericDocValues norms) throws IOException {
- for (int i = 1; i < buffer.size; ++i) {
- assert buffer.docs[i] > buffer.docs[i - 1];
- }
- for (int i = 0; i < buffer.size; ++i) {
- assert buffer.features[i] > 0;
- }
- delegate.score(buffer, norms);
- for (int i = 0; i < buffer.size; ++i) {
- float score = buffer.features[i];
- assert Float.isFinite(score);
- assert score >= 0;
- }
- }
-
@Override
public Explanation explain(Explanation freq, long norm) {
// freq in bounds
diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/BaseSimilarityTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/BaseSimilarityTestCase.java
index a220d8167e4..763bf16e691 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/BaseSimilarityTestCase.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/search/similarities/BaseSimilarityTestCase.java
@@ -17,15 +17,12 @@
package org.apache.lucene.tests.search.similarities;
import java.io.IOException;
-import java.util.Arrays;
import java.util.Random;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.document.TextField;
import org.apache.lucene.index.LeafReader;
-import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.search.CollectionStatistics;
-import org.apache.lucene.search.DocAndFloatFeatureBuffer;
import org.apache.lucene.search.Explanation;
import org.apache.lucene.search.TermStatistics;
import org.apache.lucene.search.similarities.IndriDirichletSimilarity;
@@ -36,7 +33,6 @@ import org.apache.lucene.tests.index.RandomIndexWriter;
import org.apache.lucene.tests.search.CheckHits;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.lucene.tests.util.TestUtil;
-import org.apache.lucene.util.ArrayUtil;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.SmallFloat;
@@ -525,98 +521,4 @@ public abstract class BaseSimilarityTestCase extends LuceneTestCase {
}
}
}
-
- private void doTestBulkScoring(boolean hasNorms) throws IOException {
- Random random = random();
- Similarity similarity = getSimilarity(random);
- CollectionStatistics corpus = newCorpus(random, hasNorms ? 1 : 0);
- TermStatistics term = newTerm(random, corpus);
- SimScorer scorer = similarity.scorer(random().nextFloat(5f), corpus, term);
- int freqUpperBound =
- Math.toIntExact(Math.min(term.totalTermFreq() - term.docFreq() + 1, Integer.MAX_VALUE));
- DocAndFloatFeatureBuffer buffer = new DocAndFloatFeatureBuffer();
- int iters = atLeast(3);
- for (int iter = 0; iter < iters; ++iter) {
- int size = TestUtil.nextInt(random, 0, 200);
- buffer.growNoCopy(size);
- long[] norms;
- if (hasNorms) {
- norms = new long[size];
- } else {
- norms = null;
- }
- int prevDoc = -1;
- for (int i = 0; i < size; ++i) {
- buffer.docs[i] = prevDoc + TestUtil.nextInt(random, 1, 5);
- prevDoc = buffer.docs[i];
- buffer.features[i] = TestUtil.nextInt(random, 1, freqUpperBound);
- if (hasNorms) {
- norms[i] = TestUtil.nextLong(random, 1, 255);
- }
- }
- buffer.size = size;
- NumericDocValues normValues = null;
- if (hasNorms) {
- normValues =
- new NumericDocValues() {
-
- private int index = -1;
-
- @Override
- public int nextDoc() throws IOException {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public int docID() {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public long cost() {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public int advance(int target) throws IOException {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public boolean advanceExact(int target) throws IOException {
- index = Arrays.binarySearch(buffer.docs, 0, buffer.size, target);
- assertTrue(
- "Looking up unrelated doc: "
- + target
- + " not in "
- + Arrays.toString(ArrayUtil.copyOfSubArray(buffer.docs, 0, buffer.size)),
- index >= 0);
- return true;
- }
-
- @Override
- public long longValue() throws IOException {
- return norms[index];
- }
- };
- }
-
- float[] expectedScores = new float[size];
- for (int i = 0; i < size; ++i) {
- expectedScores[i] = scorer.score(buffer.features[i], hasNorms ? norms[i] : 1L);
- }
- scorer.score(buffer, normValues);
-
- assertArrayEquals(
- expectedScores, ArrayUtil.copyOfSubArray(buffer.features, 0, buffer.size), 0f);
- }
- }
-
- public void testBulkScoringNoNorms() throws IOException {
- doTestBulkScoring(false);
- }
-
- public void testBulkScoringWithNorms() throws IOException {
- doTestBulkScoring(true);
- }
}