ChrisHegarty commented on code in PR #16682:
URL: https://github.com/apache/lucene/pull/16682#discussion_r4069898026
##########
lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene102/Lucene102BinaryQuantizedVectorsReader.java:
##########
@@ -128,10 +130,15 @@ public Lucene102BinaryQuantizedVectorsReader(
versionMeta,
VECTOR_DATA_EXTENSION,
Lucene102BinaryQuantizedVectorsFormat.VECTOR_DATA_CODEC_NAME,
- // Quantized vectors are accessed randomly from their node ID
stored in the HNSW
- // graph.
+ // how these are read is up to whoever wraps this format
state.context.withHints(
- FileTypeHint.DATA, FileDataHint.KNN_VECTORS,
DataAccessHint.RANDOM));
+ Stream.of(
+ FileTypeHint.DATA,
+ FileDataHint.KNN_VECTORS,
+
state.context.hints(DataAccessHint.class).findFirst().orElse(null),
+
state.context.hints(NoReuseHint.class).findFirst().orElse(null))
+ .filter(Objects::nonNull)
+ .toArray(IOContext.FileOpenHint[]::new)));
Review Comment:
Building on the previous comment (union), and if we had this:
```
/** Returns an IOContext with each of the given hints added, but only for a
hint type not already present */
default IOContext coalesce(FileOpenHint... hints) {
Set<Class<?>> present =
hints().stream().map(FileOpenHint::getClass).collect(toSet());
FileOpenHint[] toAdd = Stream.of(defaults)
.filter(h -> !present.contains(h.getClass()))
.toArray(FileOpenHint[]::new);
return union(toAdd);
}
```
Call sites become something like:
```
state.context
.union(FileTypeHint.DATA, FileDataHint.KNN_VECTORS)
.coalesce(accessHint)
```
##########
lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene102/Lucene102BinaryQuantizedVectorsFormat.java:
##########
@@ -141,4 +144,13 @@ public String toString() {
+ rawVectorFormat
+ ")";
}
+
+ /** The raw vectors are only read back to rescore. */
+ private static SegmentReadState rescoreOnly(SegmentReadState state) {
+ IOContext.FileOpenHint[] hints =
+ Stream.concat(state.context.hints().stream(),
Stream.of(NoReuseHint.INSTANCE))
+ .distinct()
+ .toArray(IOContext.FileOpenHint[]::new);
+ return new SegmentReadState(state, state.context.withHints(hints));
Review Comment:
Is it worth adding a method to IOContext to support this pattern?
```
default IOContext union(FileOpenHint... hints) {
return withHints(Stream.concat(hints().stream(), Stream.of(hints))
.toArray(FileOpenHint[]::new));
}
```
--
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]
