dungba88 commented on code in PR #12624:
URL: https://github.com/apache/lucene/pull/12624#discussion_r1391165022
##########
lucene/core/src/java/org/apache/lucene/util/fst/BytesStore.java:
##########
@@ -26,7 +26,8 @@
// TODO: merge with PagedBytes, except PagedBytes doesn't
// let you read while writing which FST needs
-class BytesStore extends DataOutput implements FSTReader {
+// TODO: Separate the scratch writer and reader functionality
+class BytesStore extends DataOutput implements FSTReader, Freezeable {
Review Comment:
> Could we remove this class entirely? And callers that want to write FST
and immediately use it in RAM should just use ByteBuffersDataOutput for their
scratch area?
This BytesStore class currently serves as 3 purposes:
- Acts as a scratch area writer. Their operations (copying random bytes from
one place to other, truncating, skipping the bytes) seems to be complicated to
model with other DataOutput
- Acts as a FST writer. Any other DataOutput can be used.
- Acts as a FST reader. This is tricky as DataOutput does not support read
operation, and a separate DataInput needs to be provided.
To replace the third one, I think we should keep both the second and third
as a single class, because they have to match (a ByteBufferDataOutput needs to
go with the ByteBufferRandomAccessInput). However the third purpose is only
useful for write-then-read-immediately, which I assume is not the most common
use case. Here we can either use the existing BytesStore, or create something
based on ByteBufferDataOutput as you suggested. Both are easily replaceable in
the future.
--
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]
