msfroh commented on code in PR #15722: URL: https://github.com/apache/lucene/pull/15722#discussion_r2855307486
########## lucene/core/src/java/org/apache/lucene/codecs/hnsw/PrefetchableFlatVectorScorer.java: ########## @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.codecs.hnsw; + +import java.io.IOException; +import org.apache.lucene.index.KnnVectorValues; +import org.apache.lucene.index.VectorSimilarityFunction; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.hnsw.RandomVectorScorer; +import org.apache.lucene.util.hnsw.RandomVectorScorerSupplier; +import org.apache.lucene.util.hnsw.UpdateableRandomVectorScorer; + +/** + * A {@link FlatVectorsScorer} wrapper that enables prefetching of vector data before scoring. + * + * <p>This implementation demonstrates how to use prefetch operations with KNN search to preload + * vectors into memory before distance computations, potentially improving performance by reducing + * memory access latency during bulk scoring operations. + * + * <p>The prefetching occurs in {@link PrefetchableRandomVectorScorer#bulkScore} before delegating + * to the underlying scorer. + * + * @lucene.experimental + */ +public class PrefetchableFlatVectorScorer implements FlatVectorsScorer { + + private final FlatVectorsScorer flatVectorsScorer; + + public PrefetchableFlatVectorScorer(FlatVectorsScorer flatVectorsScorer) { + this.flatVectorsScorer = flatVectorsScorer; + } + + @Override + public RandomVectorScorerSupplier getRandomVectorScorerSupplier( + VectorSimilarityFunction similarityFunction, KnnVectorValues vectorValues) + throws IOException { + return new PrefetchableRandomVectorScorerSupplier( + flatVectorsScorer.getRandomVectorScorerSupplier(similarityFunction, vectorValues)); + } + + @Override + public RandomVectorScorer getRandomVectorScorer( + VectorSimilarityFunction similarityFunction, KnnVectorValues vectorValues, float[] target) + throws IOException { + return new PrefetchableRandomVectorScorer( + (RandomVectorScorer.AbstractRandomVectorScorer) + flatVectorsScorer.getRandomVectorScorer(similarityFunction, vectorValues, target)); + } + + @Override + public RandomVectorScorer getRandomVectorScorer( + VectorSimilarityFunction similarityFunction, KnnVectorValues vectorValues, byte[] target) + throws IOException { + return new PrefetchableRandomVectorScorer( + (RandomVectorScorer.AbstractRandomVectorScorer) + flatVectorsScorer.getRandomVectorScorer(similarityFunction, vectorValues, target)); + } + + @Override + public String toString() { + return "PrefetchableFlatVectorScorer()"; + } + + /** + * A supplier that wraps another {@link RandomVectorScorerSupplier} to provide prefetchable + * scorers. + */ + static class PrefetchableRandomVectorScorerSupplier implements RandomVectorScorerSupplier { + + private final RandomVectorScorerSupplier randomVectorScorerSupplier; + + public PrefetchableRandomVectorScorerSupplier( + RandomVectorScorerSupplier randomVectorScorerSupplier) { + this.randomVectorScorerSupplier = randomVectorScorerSupplier; + } + + @Override + public UpdateableRandomVectorScorer scorer() throws IOException { + return this.randomVectorScorerSupplier.scorer(); + } + + @Override + public RandomVectorScorerSupplier copy() throws IOException { + return new PrefetchableRandomVectorScorerSupplier(randomVectorScorerSupplier.copy()); + } + } + + /** + * A {@link RandomVectorScorer} that prefetches vector data before bulk scoring operations. + * + * <p>This scorer delegates all operations to an underlying scorer, but intercepts {@link + * #bulkScore} to prefetch the required vectors before scoring. + */ + static class PrefetchableRandomVectorScorer + extends RandomVectorScorer.AbstractRandomVectorScorer { + + private final RandomVectorScorer.AbstractRandomVectorScorer randomVectorScorer; Review Comment: There were some earlier issues discussing the correct behavior for wrapper classes when the underlying abstract class might change. I think there might be some value in adding a unit test for this class similar to [TestFilterWeight](https://github.com/apache/lucene/blob/main/lucene/core/src/test/org/apache/lucene/search/TestFilterWeight.java) that uses reflection to make sure that all methods are delegated. ########## lucene/core/src/java/org/apache/lucene/codecs/hnsw/PrefetchableFlatVectorScorer.java: ########## @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.codecs.hnsw; + +import java.io.IOException; +import org.apache.lucene.index.KnnVectorValues; +import org.apache.lucene.index.VectorSimilarityFunction; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.hnsw.RandomVectorScorer; +import org.apache.lucene.util.hnsw.RandomVectorScorerSupplier; +import org.apache.lucene.util.hnsw.UpdateableRandomVectorScorer; + +/** + * A {@link FlatVectorsScorer} wrapper that enables prefetching of vector data before scoring. + * + * <p>This implementation demonstrates how to use prefetch operations with KNN search to preload + * vectors into memory before distance computations, potentially improving performance by reducing + * memory access latency during bulk scoring operations. + * + * <p>The prefetching occurs in {@link PrefetchableRandomVectorScorer#bulkScore} before delegating + * to the underlying scorer. + * + * @lucene.experimental + */ +public class PrefetchableFlatVectorScorer implements FlatVectorsScorer { + + private final FlatVectorsScorer flatVectorsScorer; + + public PrefetchableFlatVectorScorer(FlatVectorsScorer flatVectorsScorer) { + this.flatVectorsScorer = flatVectorsScorer; + } + + @Override + public RandomVectorScorerSupplier getRandomVectorScorerSupplier( + VectorSimilarityFunction similarityFunction, KnnVectorValues vectorValues) + throws IOException { + return new PrefetchableRandomVectorScorerSupplier( + flatVectorsScorer.getRandomVectorScorerSupplier(similarityFunction, vectorValues)); + } + + @Override + public RandomVectorScorer getRandomVectorScorer( + VectorSimilarityFunction similarityFunction, KnnVectorValues vectorValues, float[] target) + throws IOException { + return new PrefetchableRandomVectorScorer( + (RandomVectorScorer.AbstractRandomVectorScorer) + flatVectorsScorer.getRandomVectorScorer(similarityFunction, vectorValues, target)); + } + + @Override + public RandomVectorScorer getRandomVectorScorer( + VectorSimilarityFunction similarityFunction, KnnVectorValues vectorValues, byte[] target) + throws IOException { + return new PrefetchableRandomVectorScorer( + (RandomVectorScorer.AbstractRandomVectorScorer) + flatVectorsScorer.getRandomVectorScorer(similarityFunction, vectorValues, target)); + } + + @Override + public String toString() { + return "PrefetchableFlatVectorScorer()"; + } + + /** + * A supplier that wraps another {@link RandomVectorScorerSupplier} to provide prefetchable + * scorers. + */ + static class PrefetchableRandomVectorScorerSupplier implements RandomVectorScorerSupplier { + + private final RandomVectorScorerSupplier randomVectorScorerSupplier; + + public PrefetchableRandomVectorScorerSupplier( + RandomVectorScorerSupplier randomVectorScorerSupplier) { + this.randomVectorScorerSupplier = randomVectorScorerSupplier; + } + + @Override + public UpdateableRandomVectorScorer scorer() throws IOException { + return this.randomVectorScorerSupplier.scorer(); + } + + @Override + public RandomVectorScorerSupplier copy() throws IOException { + return new PrefetchableRandomVectorScorerSupplier(randomVectorScorerSupplier.copy()); + } Review Comment: Does this `PrefetchableRandomVectorScorerSupplier` actually do anything? I don't see it producing a `PrefetchableRandomVectorScorer`. Was the `scorer()` method supposed to do that? (But that means that `PrefetchableRandomVectorScorer` needs to implement `UpdateableRandomVectorScorer` -- maybe that's not a problem?) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
