dungba88 commented on code in PR #12624:
URL: https://github.com/apache/lucene/pull/12624#discussion_r1392710843


##########
lucene/core/src/java/org/apache/lucene/util/fst/BytesStore.java:
##########
@@ -21,12 +21,13 @@
 import java.util.List;
 import org.apache.lucene.store.DataInput;
 import org.apache.lucene.store.DataOutput;
+import org.apache.lucene.util.Accountable;
 import org.apache.lucene.util.RamUsageEstimator;
 
-// TODO: merge with PagedBytes, except PagedBytes doesn't
-// let you read while writing which FST needs
+// TODO: merge with or use PagedBytes/ByteBuffersDataOutput, as we no longer 
need to read while
+// writing
 
-class BytesStore extends DataOutput implements FSTReader {
+class BytesStore extends DataOutput implements Accountable {

Review Comment:
   Do you mean essentially use byte[] instead of byte block? I think that would 
simpler the writing logic (I was hesitated as it means we would need double RAM 
when resizing, but not too bad as it's only one node)
   
   But we still need to have the same functionality as the current BytesStore 
right? I feel like if we just remove the BytesStore down right, it's just 
moving the complexity from BytesStore to FSTCompiler, and I think having 
low-level writing in another class is better.
   
   How's about we changing the BytesStore to use byte[] instead of byte block?



-- 
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: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to