Adopted LFSR_DATA_ISBPTR flag in lfsr_data_t/lfsr_bptr_t

This takes advantage of another bit in lfsr_data_t's size field to
differentiate between normal lfsr_data_ts, and lfsr_data_ts in a bptr:

  in-RAM buffer:       on-disk data:        on-disk bptr:
  .---+---+---+---. .. .---+---+---+---. .. .---+---+---+---.
  |00|   size     |    |10|   size     |    |11|   size     |
  +---+---+---+---+ .. +---+---+---+---+    +---+---+---+---+
  |      ptr -------.  |     block     |    |     block     |
  +---+---+---+---+ |  +---+---+---+---+    +---+---+---+---+
  |    (unused)   | |  |      off      |    |      off      |
  '---+---+---+---' |  '---+---+---+---' .. +---+---+---+---+
  .---+---+---+---. |                       |     cksize    |
  |     data      |<'                       +---+---+---+---+
  :       :       :                         |     cksum     |
                                            '---+---+---+---'

Note this bit is unused even in a theoretical 16/14-bit littlefs mode.
This also leaves space for one more encoding (0b01), but I don't have
any good use for this yet. Previous ideas around an inlined
representation failed to improve anything.

This accomplishes a couple things:

1. We no longer need to return the tag in lfsr_file_lookupnext, since
   these can only be blocks or fragments.

2. We no longer need to rely on cksize=0 to determine checksummed data
   from non-checksummed data when running with LFS_CKDATACKSUMS.

This was supposed to be a relatively free optimization, but our
lfsr_data_fromslice implementation is being a bit... funky... It seems
we're right on the edge of some inline heuristic, where adding this flag
prevents lfsr_data_fromslice from being inlined, missing a number of
contextual optimizations and causing things to explode.

This can be worked around with __attribute__((always_inline)), but we
should probably revisit our data slicing macros to see if this can be
solved without a compiler specific hack. Relying on such a sensitive
function is not great:

                        code          stack          ctx
  always_inline:       36320           2584          640
  inline:              36424 (+0.3%)   2664 (+3.1%)  640 (+0.0%)

Weird inlining noise aside, this was an overall improvement. Not needing
to fetch tags in lfsr_file_lookupnext saves a bit of stack in our
hot-path, which is nice:

                        code          stack          ctx
  default before:      36460           2608          640
  default after:       36320 (-0.4%)   2584 (-0.9%)  640 (+0.0%)

Hmmm, though maybe not for ckdatacksums:

                        code          stack          ctx
  ckdatacksums before: 37628           3048          640
  ckdatacksums after:  38096 (+1.2%)   3072 (+0.8%)  640 (+0.0%)
This commit is contained in:
Christopher Haster
2025-02-04 19:57:20 -06:00
parent b115ebbac0
commit d248f70e6a
2 changed files with 99 additions and 132 deletions
+9 -7
View File
@@ -595,14 +595,15 @@ struct lfs_file_config {
// lfs_block_t tail[2];
//} lfs_mdir_t;
// either an on-disk or in-device data pointer
// either an on-disk or in-RAM data pointer
//
// note, it's enticing to make this fancier, but we benefit quite a lot
// note, it's tempting to make this fancier, but we benefit quite a lot
// from the compiler being able to aggresively optimize this struct
//
typedef struct lfsr_data {
// sign(size)=0 => in-RAM buffer
// sign(size)=1 => on-disk reference
// sign2(size)=0b00 => in-RAM buffer
// sign2(size)=0b10 => on-disk data
// sign2(size)=0b11 => on-disk data + cksum
lfs_size_t size;
union {
const uint8_t *buffer;
@@ -611,8 +612,6 @@ typedef struct lfsr_data {
lfs_size_t off;
// optional context for validating data
#ifdef LFS_CKDATACKSUMS
// cksize==0 => no checksum
// cksize!=0 => yes checksum
lfs_size_t cksize;
uint32_t cksum;
#endif
@@ -620,8 +619,11 @@ typedef struct lfsr_data {
} u;
} lfsr_data_t;
// a block pointer
// a possible block pointer
typedef struct lfsr_bptr {
// sign2(size)=0b00 => in-RAM buffer
// sign2(size)=0b10 => on-disk data
// sign2(size)=0b11 => block pointer
lfsr_data_t data;
#ifndef LFS_CKDATACKSUMS
lfs_size_t cksize;