Rerouted lfsr_file_read_'s leaf eviction -> lfsr_file_crystallize
This commit actually does two things:
1. Opportunistically marks caches as flushed if they were included in
the crystallization region in lfsr_file_crystallize
2. Reroutes lfsr_file_read_ through lfsr_file_crystallize to minimize
stack cost
---
Digging into why lazy-crystallization adds so much stack, it seems the
main reason is because lfsr_file_read_ drags in lfsr_file_flush, which
puts the entirety of the stack hot-path under both lfsr_file_read and
lfsr_file_read_.
But why are we calling lfsr_file_flush? And not just
lfsr_file_crystallize to claim the leaf? Isn't the cache flushed in
lfsr_file_read before reading?
The one concerning case is when reads bypass the cache (read >
cache_size). With cache-bypassing reads, it's entirely possible for
lfsr_file_read_ to end up with unflushed data. lfsr_file_read's logic
gives the cache priority in this case, so it's not like we're going to
read outdated data or anything, but if we crystallize without flushing
we risk wasting erased-state that will need to be recrystallized later.
What's extra humorous is our crystallization logic _does_ correctly
write out the cache, it just doesn't clear the LFS_o_UNFLUSH bit because
it doesn't know if progress has been made.
So to avoid this, all we need to do is add an explicit check to
lfsr_file_crystallize that clears the LFS_o_UNFLUSH bit if our cache
ends up written out as a part of crystallization.
Note this is slightly more powerful than lfsr_file_flush, since we don't
_need_ to flush the cache if it's not in our crystallization region.
As an extra plus this affects all lfsr_file_crystallize calls, so now
lfsr_file_truncate/fruncate also avoid unnecessary recrystallization.
That's some good code reuse right there!
---
Long story short, rerouting lfsr_file_read_ through
lfsr_file_crystallize moves it off the stack hot-path, bringing our
stack down to almost pre-lazy-crystallization levels:
code stack ctx
before: 37140 2304 636
after: 37184 (+0.1%) 2288 (-0.7%) 636 (+0.0%)
At a code cost, but this code also allows lfsr_file_read/truncate/
fruncate to avoid recrystallization with opportunistic flushes in cases
where we need to discard file->leaf.
This commit is contained in:
@@ -11724,15 +11724,21 @@ static int lfsr_file_lookup(lfs_t *lfs, const lfsr_file_t *file,
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// needed in lfsr_file_read_
|
||||||
|
static int lfsr_file_crystallize(lfs_t *lfs, lfsr_file_t *file);
|
||||||
|
|
||||||
static lfs_ssize_t lfsr_file_read_(lfs_t *lfs, lfsr_file_t *file,
|
static lfs_ssize_t lfsr_file_read_(lfs_t *lfs, lfsr_file_t *file,
|
||||||
lfs_off_t pos, uint8_t *buffer, lfs_size_t size) {
|
lfs_off_t pos, uint8_t *buffer, lfs_size_t size) {
|
||||||
// need to fetch a new leaf?
|
// need to fetch a new leaf?
|
||||||
if (!(pos >= file->leaf.pos
|
if (!(pos >= file->leaf.pos
|
||||||
&& pos < file->leaf.pos + file->leaf.weight)) {
|
&& pos < file->leaf.pos + file->leaf.weight)) {
|
||||||
// leaf in use? we need to flush it
|
// leaf in use? we need to crystallize/graft it
|
||||||
|
//
|
||||||
|
// it would be easier to just call lfsr_file_flush here, but
|
||||||
|
// we don't want to drag in the extra stack usage
|
||||||
if (lfsr_o_isungraft(file->b.o.flags)
|
if (lfsr_o_isungraft(file->b.o.flags)
|
||||||
|| lfsr_o_isuncryst(file->b.o.flags)) {
|
|| lfsr_o_isuncryst(file->b.o.flags)) {
|
||||||
int err = lfsr_file_flush(lfs, file);
|
int err = lfsr_file_crystallize(lfs, file);
|
||||||
if (err) {
|
if (err) {
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
@@ -12398,6 +12404,10 @@ static int lfsr_file_crystallize_(lfs_t *lfs, lfsr_file_t *file,
|
|||||||
}
|
}
|
||||||
|
|
||||||
static int lfsr_file_crystallize(lfs_t *lfs, lfsr_file_t *file) {
|
static int lfsr_file_crystallize(lfs_t *lfs, lfsr_file_t *file) {
|
||||||
|
bool flushable = (
|
||||||
|
file->cache.pos
|
||||||
|
>= file->leaf.pos + lfsr_bptr_size(&file->leaf.bptr));
|
||||||
|
|
||||||
// finish crystallizing
|
// finish crystallizing
|
||||||
if (lfsr_o_isuncryst(file->b.o.flags)) {
|
if (lfsr_o_isuncryst(file->b.o.flags)) {
|
||||||
// uncrystallized files must be unsynced
|
// uncrystallized files must be unsynced
|
||||||
@@ -12432,6 +12442,13 @@ static int lfsr_file_crystallize(lfs_t *lfs, lfsr_file_t *file) {
|
|||||||
file->b.o.flags &= ~LFS_o_UNGRAFT;
|
file->b.o.flags &= ~LFS_o_UNGRAFT;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// eagerly mark as flushed if this included all of our cache
|
||||||
|
if (flushable
|
||||||
|
&& file->leaf.pos + lfsr_bptr_size(&file->leaf.bptr)
|
||||||
|
>= file->cache.pos + file->cache.size) {
|
||||||
|
file->b.o.flags &= ~LFS_o_UNFLUSH;
|
||||||
|
}
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user