Use a BufReader in bdk_file_store#1293
Conversation
* Wrap file reader with `BufReader`. This reduces calls to `read`. * Wrap file reader with `CountingReader`. This counts the bytes read by the underlying reader. We can rewind without seeking first.
This test simulates a situation where the last write to the db is short. Aggregating the changeset after reopening the file should return an error (which includes a partially-aggregated changeset) containing an aggregation of changesets that were fully written. At this point, the test re-writes the final changeset (and this time it successfully writes in full). The file should be recoverable with all changesets, including the last one.
LLFourn
force-pushed
the
llfourn_file_store_performance
branch
from
2fd8d53 to
aa1b16a
Compare
| } | ||
| }; | ||
| let result = read_one(self.db_file.as_mut()?, self.start_pos.take()); | ||
| if result.is_err() { | ||
| self.db_file = None; | ||
| } | ||
| result.transpose() | ||
| } | ||
| } | ||
|
|
||
| impl<'t, T> Drop for EntryIter<'t, T> { | ||
| fn drop(&mut self) { | ||
| if let Some(r) = self.db_file.as_mut() { | ||
| // This syncs the underlying file's offset with the buffer's position. This way, no data | ||
| // is lost with future reads. | ||
| let _ = r.stream_position(); | ||
| } | ||
| } | ||
| })() | ||
| .transpose() | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we still need a Drop implementation that syncs the underlying file's offset. However, we should use self.seek(SeekFrom::Current(0)).
There was a problem hiding this comment.
Can we write a test that demonstrates this need? I think I understand the rationale I'll see if I can come up one.
[EDIT] or maybe we can use a BufReader in the main type (not just the iterator).
There was a problem hiding this comment.
@LLFourn I don't think using a BufReader with the internal file getting written to is a good idea. We'll need to do some crazy stuff with .get_mut and flush the buffer with every write.
I thought that the test I added demonstrated the need for the Drop impl but clearly not. We need a test to write after a failed read without closing the file in-between.
|
This is continued in #1270 |
2 participants
PR over #1270 to simplify it. Key observation is that you can use stream_position in
BufReaderwithout making a system call. I think @evanlinjin introduced CountingReader to keep track of where we are in the stream butBufReaderalready does this (correct me if I'm wrong).Take a close look please @evanlinjin. It passes the tests but I might be missing something.