Github/littlefs
1
Fork 0
Code Actions Activity

Reverted most of the crammed 15-bit lfsr_attr_t representation

Browse Source 
I think this was becoming too over-engineered. That and if you're
fighting the compiler too much, sometimes it's best to just let the
compiler win.

There is also the minor benefit of not needing to worry about uint16_t
overflows in lfsr_cat_t. One concern is it's easy for missed lfsr_data_t
indirections to go unnoticed.

This reverts lfsr_attr_t from 3 words -> 4 words, but allows easy
mapping to the lfsr_cat_t part of the lfsr_attr_t:

  lfsr_tag_t               lfsr_attr_t
  .---+---.                .---+---+---+---. . . .---+---+---+---.
  |  tag  |--------------->|  tag  |       |     |  tag  |       |
  '---+---'                +---+---+---+---+     +---+---+---+---+
                    .----->|     delta     |     |     delta     |
  lfsr_srid_t       |      +---+---+---+---+     +---+---+---+---+
  .---+---+---+---. | .--->|c|    size     |     |      cat      |
  |     delta     |-' |    +---+---+---+---+     |               |
  '---+---+---+---'   | .->|      ptr      |     |               |
                      | |  '---+---+---+---' . . '---+---+---+---'
  lfsr_cat_t          | |
  .---+---+---+---.   | |
  |c|    size     |---' |
  +---+---+---+---+     |
  |      ptr      |-----'
  '---+---+---+---'

This doesn't impact stack as much as you would expect due to compiler
overhead around inline functions operating on multi-word structs:

  before 15-bit: 33672           2776
  best 15-bit:   33900 (+0.7%)   2736 (-1.4%)
  prev 15-bit:   34486 (+2.4%)   2912 (+4.9%)
  after:         33672 (+0.0%)   2744 (-1.2%)

I think the only big thing not reverted was switching lfsr_cat_t from
storing the sum of the lfsr_data_ts sizes to just storing the number of
lfsr_data_ts. This is cheaper to initialize (because inline functions
are weirdly costly!), cheaper to iterate over, and we don't actually
need lfsr_cat_size that often, unlike lfsr_data_size.

Eventually we'll probably need to revisit this to optimize 31/15-bit,
etc, littlefs configurations. But that's a problem for another day...
This commit is contained in:
Christopher Haster
2024-05-08 00:49:56 -05:00
parent 7e4d8c1df2
commit 2383e7f9f9
1 changed files with 52 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+52 -72
View File
@@ -1416,20 +1416,19 @@ typedef struct lfsr_cat {
// sign(size)=0 => single in-RAM buffer
// sign(size)=1 => multiple concatenated datas
union {
lfs_size_t size;
struct {
lfsr_tag_t tag;
uint16_t size;
lfs_size_t size;
const uint8_t *buffer;
} buf;
struct {
lfsr_tag_t tag;
uint16_t size;
lfs_size_t size;
const lfsr_data_t *datas;
} cat;
} u;
} lfsr_cat_t;
#define LFSR_CAT_ISCAT 0x8000
#define LFSR_CAT_ISCAT 0x80000000
#define LFSR_CAT_NULL() \
((lfsr_cat_t){ \
@@ -1453,11 +1452,11 @@ typedef struct lfsr_cat {
// cat helpers
static inline bool lfsr_cat_isbuf(lfsr_cat_t cat) {
return !(cat.u.buf.size & LFSR_CAT_ISCAT);
return !(cat.u.size & LFSR_CAT_ISCAT);
}
static inline bool lfsr_cat_iscat(lfsr_cat_t cat) {
return cat.u.buf.size & LFSR_CAT_ISCAT;
return cat.u.size & LFSR_CAT_ISCAT;
}
static inline lfsr_data_t lfsr_cat_data(lfsr_cat_t cat) {
@@ -1466,19 +1465,19 @@ static inline lfsr_data_t lfsr_cat_data(lfsr_cat_t cat) {
return LFSR_DATA_BUF(cat.u.buf.buffer, cat.u.buf.size);
}
static inline uint16_t lfsr_cat_count(lfsr_cat_t cat) {
static inline lfs_size_t lfsr_cat_count(lfsr_cat_t cat) {
LFS_ASSERT(lfsr_cat_iscat(cat));
return cat.u.buf.size & ~LFSR_CAT_ISCAT;
return cat.u.size & ~LFSR_CAT_ISCAT;
}
static inline lfs_size_t lfsr_cat_size(lfsr_cat_t cat) {
// this gets a bit complicated for concatenated data
if (lfsr_cat_isbuf(cat)) {
return cat.u.buf.size;
return cat.u.size;
} else {
uint16_t count = lfsr_cat_count(cat);
lfs_size_t count = lfsr_cat_count(cat);
lfs_size_t size = 0;
for (uint16_t i = 0; i < count; i++) {
for (lfs_size_t i = 0; i < count; i++) {
size += lfsr_data_size(cat.u.cat.datas[i]);
}
return size;
@@ -1546,8 +1545,8 @@ static int lfsr_bd_progcat(lfs_t *lfs,
// indirect concatenated data?
} else {
uint16_t count = lfsr_cat_count(cat);
for (uint16_t i = 0; i < count; i++) {
lfs_size_t count = lfsr_cat_count(cat);
for (lfs_size_t i = 0; i < count; i++) {
int err = lfsr_bd_progdata(lfs, block, off, cat.u.cat.datas[i],
cksum_);
if (err) {
@@ -1563,33 +1562,17 @@ static int lfsr_bd_progcat(lfs_t *lfs,
// operations on attribute lists
// needed for lfsr_attr
typedef struct lfsr_shrubcommit lfsr_shrubcommit_t;
typedef struct lfsr_attr {
lfsr_tag_t tag;
lfsr_srid_t delta;
union {
lfsr_tag_t tag;
lfsr_cat_t cat;
} u;
lfsr_cat_t cat;
} lfsr_attr_t;
#define LFSR_ATTR(_tag, _delta, _cat) \
((lfsr_attr_t){ \
.delta=_delta, \
.u.cat=lfsr_cat_tag(_cat, _tag)})
static inline lfsr_cat_t lfsr_cat_tag(lfsr_cat_t cat, lfsr_tag_t tag) {
cat.u.buf.tag = tag;
return cat;
}
((lfsr_attr_t){_tag, _delta, _cat})
#define LFSR_ATTR_NOOP() \
((const lfsr_attr_t){ \
.delta=_delta, \
.u.cat.u.buf.tag=LFSR_TAG_NULL, \
.u.cat.u.buf.size=0, \
.u.cat.u.buf.buffer=NULL})
LFSR_ATTR(LFSR_TAG_NULL, 0, LFSR_CAT_NULL())
// create an attribute list
#define LFSR_ATTRS(...) \
@@ -1601,25 +1584,26 @@ static inline lfsr_cat_t lfsr_cat_tag(lfsr_cat_t cat, lfsr_tag_t tag) {
// a move of all attrs from an mdir entry
#define LFSR_ATTR_MOVE(_tag, _delta, _mdir) \
((const lfsr_attr_t){ \
.tag=_tag, \
.delta=_delta, \
.u.cat.u.buf.tag=_tag, \
.u.cat.u.buf.buffer=(const void*)(const lfsr_mdir_t*){_mdir}})
.cat.u.buf.buffer=(const void*)(const lfsr_mdir_t*){_mdir}})
// a grm update, note this is mutable! we may update the grm during
// mdir commits
#define LFSR_ATTR_GRM(_tag, _delta, _grm) \
((const lfsr_attr_t){ \
.tag=_tag, \
.delta=_delta, \
.u.cat.u.buf.tag=_tag, \
.u.cat.u.buf.buffer=(const void*)(const lfsr_grm_t*){_grm}})
.cat.u.buf.buffer=(const void*)(const lfsr_grm_t*){_grm}})
// writing to an unrelated trunk in the rbyd
typedef struct lfsr_shrubcommit lfsr_shrubcommit_t;
#define LFSR_ATTR_SHRUBCOMMIT(_tag, _delta, \
_shrub, _rid, _attrs, _attr_count) \
((const lfsr_attr_t){ \
.tag=_tag, \
.delta=_delta, \
.u.cat.u.buf.tag=_tag, \
.u.cat.u.buf.buffer=(const void*)&(const lfsr_shrubcommit_t){ \
.cat.u.buf.buffer=(const void*)&(const lfsr_shrubcommit_t){ \
.shrub=_shrub, \
.rid=_rid, \
.attrs=_attrs, \
@@ -1627,35 +1611,35 @@ static inline lfsr_cat_t lfsr_cat_tag(lfsr_cat_t cat, lfsr_tag_t tag) {
#define LFSR_ATTR_SHRUBTRUNK(_tag, _delta, _shrub) \
((const lfsr_attr_t){ \
.tag=_tag, \
.delta=_delta, \
.u.cat.u.buf.tag=_tag, \
.u.cat.u.buf.buffer=(const void*)(const lfsr_shrub_t*){_shrub}})
.cat.u.buf.buffer=(const void*)(const lfsr_shrub_t*){_shrub}})
// some helpers
static inline bool lfsr_attr_isnoop(const lfsr_attr_t *attr) {
return !attr->u.tag && attr->delta == 0;
return !attr->tag && attr->delta == 0;
}
static inline bool lfsr_attr_isinsert(const lfsr_attr_t *attr) {
return !lfsr_tag_isgrow(attr->u.tag) && attr->delta > 0;
return !lfsr_tag_isgrow(attr->tag) && attr->delta > 0;
}
static inline lfsr_grm_t *lfsr_attr_grm(const lfsr_attr_t *attr) {
return (lfsr_grm_t*)attr->u.cat.u.buf.buffer;
return (lfsr_grm_t*)attr->cat.u.buf.buffer;
}
static inline const lfsr_mdir_t *lfsr_attr_mdir(const lfsr_attr_t *attr) {
return (lfsr_mdir_t*)attr->u.cat.u.buf.buffer;
return (lfsr_mdir_t*)attr->cat.u.buf.buffer;
}
static inline const lfsr_shrubcommit_t *lfsr_attr_shrubcommit(
const lfsr_attr_t *attr) {
return (const lfsr_shrubcommit_t*)attr->u.cat.u.buf.buffer;
return (const lfsr_shrubcommit_t*)attr->cat.u.buf.buffer;
}
static inline const lfsr_shrub_t *lfsr_attr_shrubtrunk(
const lfsr_attr_t *attr) {
return (const lfsr_shrub_t*)attr->u.cat.u.buf.buffer;
return (const lfsr_shrub_t*)attr->cat.u.buf.buffer;
}
@@ -3601,7 +3585,7 @@ static int lfsr_rbyd_appendattrs(lfs_t *lfs, lfsr_rbyd_t *rbyd,
&& (lfs_size_t)(rid + 1) <= (lfs_size_t)end_rid) {
int err = lfsr_rbyd_appendattr(lfs, rbyd,
rid - lfs_smax32(start_rid, 0),
attrs[i].u.tag, attrs[i].delta, attrs[i].u.cat);
attrs[i].tag, attrs[i].delta, attrs[i].cat);
if (err) {
return err;
}
@@ -5984,13 +5968,13 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir,
// ignore any gstate tags here, these need to be handled
// specially by upper-layers
if (attrs[i].u.tag == LFSR_TAG_GRM) {
if (attrs[i].tag == LFSR_TAG_GRM) {
// do nothing
// move tags copy over any tags associated with the source's rid
// TODO can this be deduplicated with lfsr_mdir_compact__ more?
// it _really_ wants to be deduplicated
} else if (attrs[i].u.tag == LFSR_TAG_MOVE) {
} else if (attrs[i].tag == LFSR_TAG_MOVE) {
// weighted moves are not supported
LFS_ASSERT(attrs[i].delta == 0);
const lfsr_mdir_t *mdir__ = lfsr_attr_mdir(&attrs[i]);
@@ -6112,15 +6096,15 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir,
// shrub tags append a set of attributes to an unrelated trunk
// in our rbyd
} else if (attrs[i].u.tag == LFSR_TAG_SHRUBALLOC
|| attrs[i].u.tag == LFSR_TAG_SHRUBCOMMIT) {
} else if (attrs[i].tag == LFSR_TAG_SHRUBALLOC
|| attrs[i].tag == LFSR_TAG_SHRUBCOMMIT) {
const lfsr_shrubcommit_t *bshrubcommit
= lfsr_attr_shrubcommit(&attrs[i]);
// SHRUBALLOC is roughly the same as SHRUBCOMMIT but also
// resets the shrub, we need to do this here so bshrub root
// extensions are atomic
if (attrs[i].u.tag == LFSR_TAG_SHRUBALLOC) {
if (attrs[i].tag == LFSR_TAG_SHRUBALLOC) {
bshrubcommit->shrub->blocks[0] = rbyd_.blocks[0];
bshrubcommit->shrub->trunk = LFSR_RBYD_ISSHRUB | 0;
bshrubcommit->shrub->weight = 0;
@@ -6140,12 +6124,12 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir,
//
// TODO should we preserve mode for all of these?
// TODO should we do the same for sprouts?
} else if (lfsr_tag_key(attrs[i].u.tag) == LFSR_TAG_SHRUBTRUNK) {
} else if (lfsr_tag_key(attrs[i].tag) == LFSR_TAG_SHRUBTRUNK) {
const lfsr_shrub_t *shrub = lfsr_attr_shrubtrunk(&attrs[i]);
int err = lfsr_rbyd_appendattr(lfs, &rbyd_,
rid - lfs_smax32(start_rid, 0),
lfsr_tag_mode(attrs[i].u.tag) | LFSR_TAG_BSHRUB,
lfsr_tag_mode(attrs[i].tag) | LFSR_TAG_BSHRUB,
attrs[i].delta,
// note we use the staged trunk here
LFSR_CAT_SHRUB(shrub));
@@ -6155,11 +6139,11 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir,
// write out normal tags normally
} else {
LFS_ASSERT(!lfsr_tag_isinternal(attrs[i].u.tag));
LFS_ASSERT(!lfsr_tag_isinternal(attrs[i].tag));
int err = lfsr_rbyd_appendattr(lfs, &rbyd_,
rid - lfs_smax32(start_rid, 0),
attrs[i].u.tag, attrs[i].delta, attrs[i].u.cat);
attrs[i].tag, attrs[i].delta, attrs[i].cat);
if (err) {
return err;
}
@@ -6613,7 +6597,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// parse out any pending gstate, these will get automatically
// xored with on-disk gdeltas in lower-level functions
for (lfs_size_t i = 0; i < attr_count; i++) {
if (attrs[i].u.tag == LFSR_TAG_GRM) {
if (attrs[i].tag == LFSR_TAG_GRM) {
// encode to disk
lfsr_grm_t *grm = lfsr_attr_grm(&attrs[i]);
lfsr_gdelta_xorgrm(lfs, lfs->grm_d, LFSR_GRM_DSIZE, grm);
@@ -6890,7 +6874,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// patch any pending grms
for (lfs_size_t i = 0; i < attr_count; i++) {
if (attrs[i].u.tag == LFSR_TAG_GRM) {
if (attrs[i].tag == LFSR_TAG_GRM) {
// Assuming we already xored our gdelta with the grm, we first
// need to xor the grm out of the gdelta. We can't just zero
// the gdelta because we may have picked up extra gdelta from
@@ -7032,7 +7016,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
lfsr_smid_t mid = mdir->mid;
for (lfs_size_t i = 0; i < attr_count; i++) {
// update any gstate changes
if (attrs[i].u.tag == LFSR_TAG_GRM) {
if (attrs[i].tag == LFSR_TAG_GRM) {
lfs->grm = *lfsr_attr_grm(&attrs[i]);
// keep track of the exact encoding on-disk
@@ -7046,7 +7030,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
&& o->mdir.mid >= mid) {
// replaced?
if (o->mdir.mid == mid - attrs[i].delta
&& lfsr_tag_issup(attrs[i].u.tag)) {
&& lfsr_tag_issup(attrs[i].tag)) {
o->flags |= LFS_F_ZOMBIE
| LFS_F_UNSYNC
| LFS_O_DESYNC;
@@ -10203,11 +10187,11 @@ static int lfsr_bshrub_commit(lfs_t *lfs, lfsr_file_t *file,
lfs_size_t commit_estimate = 0;
for (lfs_size_t i = 0; i < attr_count; i++) {
// only include tag overhead if tag is not a grow/rm tag
if (!lfsr_tag_isgrow(attrs[i].u.tag)
&& !lfsr_tag_isrm(attrs[i].u.tag)) {
if (!lfsr_tag_isgrow(attrs[i].tag)
&& !lfsr_tag_isrm(attrs[i].tag)) {
commit_estimate += lfs->attr_estimate;
}
commit_estimate += lfsr_cat_size(attrs[i].u.cat);
commit_estimate += lfsr_cat_size(attrs[i].cat);
}
// does our estimate exceed our shrub_size? need to recalculate an
@@ -11405,10 +11389,7 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
lfsr_attr_t attrs[2];
lfs_size_t attr_count = 0;
lfsr_data_t name_data;
union {
lfsr_data_t data;
uint8_t buf[LFSR_BTREE_DSIZE];
} data;
uint8_t buf[LFSR_BTREE_DSIZE];
// not created yet? need to convert orphan to normal file
if (lfsr_f_isorphan(file->m.flags)) {
@@ -11434,10 +11415,9 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
LFSR_CAT_NULL());
// small file inlined in mdir?
} else if (lfsr_f_isunflush(file->m.flags)) {
data.data = LFSR_DATA_BUF(file->buffer, file->buffer_size);
attrs[attr_count++] = LFSR_ATTR(
LFSR_TAG_SUB | LFSR_TAG_DATA, 0,
LFSR_CAT_DATA(&data.data));
LFSR_CAT_BUF(file->buffer, file->buffer_size));
// bshrub?
} else if (lfsr_bshrub_isbshrub(&file->m.mdir, &file->bshrub)) {
attrs[attr_count++] = LFSR_ATTR_SHRUBTRUNK(
@@ -11447,7 +11427,7 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
} else if (lfsr_bshrub_isbtree(&file->m.mdir, &file->bshrub)) {
attrs[attr_count++] = LFSR_ATTR(
LFSR_TAG_SUB | LFSR_TAG_BTREE, 0,
lfsr_cat_frombtree(&file->bshrub.u.btree, data.buf));
lfsr_cat_frombtree(&file->bshrub.u.btree, buf));
} else {
LFS_UNREACHABLE();
}