jimczi commented on code in PR #16905:
URL: https://github.com/apache/lucene/pull/16905#discussion_r4377011984
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsReader.java:
##########
@@ -752,14 +752,15 @@ private synchronized IndexInput sequentialFieldsStream()
throws IOException {
return sequentialFieldsStream;
}
- /** Returns the context of the data file with sequential access. */
+ /** Returns the context merges read the data file with. */
private IOContext sequentialContext() {
Review Comment:
Renamed to `mergeInstance…`, since `getMergeInstance` is also how sequential
reads get this today. We can add a proper search-side API for that in a
follow-up.
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsReader.java:
##########
@@ -75,8 +78,8 @@ public final class Lucene99FlatVectorsReader extends
FlatVectorsReader {
private final String vectorDataFN;
// the reader this one was created from
private final Lucene99FlatVectorsReader original;
- // the input for sequential reads, opened lazily and closed with this reader
- private IndexInput sequentialVectorData;
+ // the inputs for other contexts, closed with this reader, guarded by this
+ private final Map<IOContext, IndexInput> otherVectorData = new HashMap<>();
Review Comment:
Good catch, the merge contexts never matched. I dropped the map and went
back to what main does, just with one input for merges and one for sequential
values.
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsReader.java:
##########
@@ -212,40 +215,44 @@ public void checkIntegrity(MergePolicy.OneMerge merge)
throws IOException {
@Override
public FlatVectorsReader getMergeInstance() throws IOException {
- IndexInput data = original.sequentialVectorData();
+ IndexInput data =
original.vectorDataFor(IOContext.merge().withHints(sequentialHints()));
return data == vectorData ? this : new Lucene99FlatVectorsReader(this,
data.clone());
}
- /** Returns the input for sequential reads, opening the data file again if
needed. */
- private synchronized IndexInput sequentialVectorData() throws IOException {
+ /** Returns an input efficient for {@code context}, opening the data file
again if none is. */
+ private synchronized IndexInput vectorDataFor(IOContext context) throws
IOException {
assert original == this;
if (closed) {
throw new AlreadyClosedException("this reader is closed");
}
- if (sequentialVectorData == null) {
- IOContext sequential = sequentialContext();
- if (vectorData.isEfficientFor(sequential)) {
- sequentialVectorData = vectorData;
- } else {
- try {
- sequentialVectorData = directory.openInput(vectorDataFN, sequential);
- } catch (FileNotFoundException | NoSuchFileException _) {
- // an open reader outlives its files, so read through the input it
already holds
- sequentialVectorData = vectorData;
- }
+ if (vectorData.isEfficientFor(context)) {
+ return vectorData;
+ }
+ IndexInput data = otherVectorData.get(context);
+ if (data != null) {
+ return data;
+ }
+ for (IndexInput other : otherVectorData.values()) {
+ if (other.isEfficientFor(context)) {
+ return other;
Review Comment:
Gone with the map.
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsReader.java:
##########
@@ -212,40 +215,44 @@ public void checkIntegrity(MergePolicy.OneMerge merge)
throws IOException {
@Override
public FlatVectorsReader getMergeInstance() throws IOException {
- IndexInput data = original.sequentialVectorData();
+ IndexInput data =
original.vectorDataFor(IOContext.merge().withHints(sequentialHints()));
return data == vectorData ? this : new Lucene99FlatVectorsReader(this,
data.clone());
}
- /** Returns the input for sequential reads, opening the data file again if
needed. */
- private synchronized IndexInput sequentialVectorData() throws IOException {
+ /** Returns an input efficient for {@code context}, opening the data file
again if none is. */
+ private synchronized IndexInput vectorDataFor(IOContext context) throws
IOException {
assert original == this;
if (closed) {
throw new AlreadyClosedException("this reader is closed");
}
- if (sequentialVectorData == null) {
- IOContext sequential = sequentialContext();
- if (vectorData.isEfficientFor(sequential)) {
- sequentialVectorData = vectorData;
- } else {
- try {
- sequentialVectorData = directory.openInput(vectorDataFN, sequential);
- } catch (FileNotFoundException | NoSuchFileException _) {
- // an open reader outlives its files, so read through the input it
already holds
- sequentialVectorData = vectorData;
- }
+ if (vectorData.isEfficientFor(context)) {
+ return vectorData;
+ }
+ IndexInput data = otherVectorData.get(context);
+ if (data != null) {
+ return data;
+ }
+ for (IndexInput other : otherVectorData.values()) {
+ if (other.isEfficientFor(context)) {
+ return other;
}
}
- return sequentialVectorData;
+ try {
+ data = directory.openInput(vectorDataFN, context);
+ } catch (FileNotFoundException | NoSuchFileException _) {
+ // an open reader outlives its files, so read through the input it
already holds
+ data = vectorData;
+ }
+ otherVectorData.put(context, data);
+ return data;
}
- /** Returns the context of the data file with sequential access. */
- private IOContext sequentialContext() {
- return dataContext.withHints(
- Stream.concat(
- dataContext.hints().stream()
- .filter(hint -> hint instanceof DataAccessHint == false),
- Stream.of(DataAccessHint.SEQUENTIAL))
- .toArray(IOContext.FileOpenHint[]::new));
+ /** Returns the hints of the data file with sequential access. */
+ private IOContext.FileOpenHint[] sequentialHints() {
+ return Stream.concat(
+ dataContext.hints().stream().filter(hint -> hint instanceof
DataAccessHint == false),
Review Comment:
Done, thanks.
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsReader.java:
##########
@@ -385,17 +395,21 @@ public RandomVectorScorer getRandomVectorScorer(String
field, short[] target) th
@Override
public void close() throws IOException {
- IndexInput sequential;
+ List<IndexInput> toClose = new ArrayList<>();
synchronized (this) {
if (closed) {
return;
}
closed = true;
- sequential =
- original == this && sequentialVectorData != vectorData ?
sequentialVectorData : null;
- sequentialVectorData = null;
+ toClose.add(vectorData);
+ for (IndexInput other : otherVectorData.values()) {
+ if (other != vectorData) {
+ toClose.add(other);
Review Comment:
Done.
--
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]
