Github/littlefs
1
Fork 0
Code Actions Activity

Attempted to better reuse lfsr_bd_readnext in lfsr_bd_read

Browse Source 
lfsr_bd_readnext and lfsr_bd_read are almost the same function, with the
significant exception of cache-bypassing reads.

Bypassing reads are an interesting optimization in littlefs. Since we're
dealing with very constrained amounts of RAM, it's not uncommon for read
calls to have more RAM available than our internal caches. In this case
bypassing the cache 1. avoids copies, 2. reduces bus transaction, and 3.
leaves data in the rcache which may be useful for ongoing smaller
queries.

But bypassing reads make no sense for lfsr_bd_readnext, since
lfsr_bd_readnext calls have no buffer by definition.

This leads to a bit of a mess when you try to make lfsr_bd_read call
lfsr_bd_readnext, bypassing reads are lfsr_bd_read specific, but we need
to check for rcache/pcache prioritization first, which is the same in
both lfsr_bd_read and lfsr_bd_readnext.

The solution here is to duplicate the rcache/pcache prioritization
checks as a precondition for bypassing reads, at least deduplicating the
actual rcache/pcache memcpy. This isn't the greatest because memcpy is
actually pretty cheap in terms of code cost. But I don't see a better
organization.

The result is less code savings than expected.

Unfortunately this also comes with a high stack cost, just because of
the additional read->readnext stack frame. lfsr_bd_read is usually the
leaf on the hot path stack-wise, making the worst-case stack quite
sensitive to any changes to this function:

                      code          stack
  before readnext:   33584           2792
  dup read/readnext: 33804 (+0.7%)   2808 (+0.7%)
  rec read/readnext: 33744 (+0.5%)   2872 (+2.9%)
This commit is contained in:
Christopher Haster
2024-02-20 00:52:04 -06:00
parent d74574ed86
commit 88110c95be
1 changed files with 19 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+19 -51
View File
@@ -237,42 +237,18 @@ static int lfsr_bd_read(lfs_t *lfs,
uint8_t *buffer_ = buffer;
lfs_size_t size_ = size;
while (size_ > 0) {
// already in pcache?
if (block == lfs->pcache.block
&& off_ < lfs->pcache.off + lfs->pcache.size
&& off_ >= lfs->pcache.off) {
lfs_size_t d = lfs_min(
size_,
lfs->pcache.size - (off_-lfs->pcache.off));
memcpy(buffer_, &lfs->pcache.buffer[off_-lfs->pcache.off], d);
off_ += d;
hint_ -= d;
buffer_ += d;
size_ -= d;
continue;
}
// already in rcache?
if (block == lfs->rcache.block
&& off_ < lfs->rcache.off + lfs->rcache.size
&& off_ >= lfs->rcache.off) {
lfs_size_t d = lfs_min(
size_,
lfs->rcache.size - (off_-lfs->rcache.off));
memcpy(buffer_, &lfs->rcache.buffer[off_-lfs->rcache.off], d);
off_ += d;
hint_ -= d;
buffer_ += d;
size_ -= d;
continue;
}
// bypass cache?
if (size_ >= hint_
&& off_ % lfs->cfg->read_size == 0
&& size_ >= lfs->cfg->read_size) {
&& size_ >= lfs->cfg->read_size
// pcache takes priority
&& !(block == lfs->pcache.block
&& off_ < lfs->pcache.off + lfs->pcache.size
&& off_ >= lfs->pcache.off)
// rcache takes priority
&& !(block == lfs->rcache.block
&& off_ < lfs->rcache.off + lfs->rcache.size
&& off_ >= lfs->rcache.off)) {
lfs_size_t d = lfs_aligndown(size_, lfs->cfg->read_size);
int err = lfsr_bd_read_(lfs, block, off_, buffer_, d);
if (err) {
@@ -286,28 +262,20 @@ static int lfsr_bd_read(lfs_t *lfs,
continue;
}
// drop rcache in case read fails
lfsr_cache_drop(&lfs->rcache);
// load to cache, first condition can no longer fail
lfs_size_t off__ = lfs_aligndown(off_, lfs->cfg->read_size);
// watch out for overflow when hint_=-1
lfs_size_t size__ = lfs_alignup(
(off_-off__) + lfs_min(
lfs_max(size_, hint_),
lfs_min(
lfs->cfg->cache_size - (off_-off__),
lfs->cfg->block_size - off_)),
lfs->cfg->read_size);
int err = lfsr_bd_read_(lfs, block, off__,
lfs->rcache.buffer, size__);
const uint8_t *buffer__;
lfs_size_t size__;
int err = lfsr_bd_readnext(lfs, block, off_, hint_, size_,
&buffer__, &size__);
if (err) {
return err;
}
lfs->rcache.block = block;
lfs->rcache.off = off__;
lfs->rcache.size = size__;
memcpy(buffer_, buffer__, size__);
off_ += size__;
hint_ -= size__;
buffer_ += size__;
size_ -= size__;
}
return 0;