Dropped "has id" bit encoding in favor of invalid id

I've been trying to keep tag types organized with an encoding that hints
if a tag uses its id field for file ids. However this seem to have been
a mistake. Using a null id of 0x3ff greatly simplified quite a bit of
the logic around managing file related tags.

The downside is one less id we can use, but if we look at the encoding
cost, donating one full bit costs us 2^9 id permutations vs 1 id
permutation. So even if we had a perfect encoding it's in our favor to
use a null id. The cost of null ids is code size, but with the
complexity around figuring out if a type used it's id or not it just
works out better to use a null id.
This commit is contained in:
Christopher Haster
2018-07-09 14:51:57 -05:00
parent b7bd34f461
commit fd121dc2e2
3 changed files with 16 additions and 23 deletions

25
lfs.c
View File

@@ -439,10 +439,6 @@ static inline bool lfs_tag_isuser(lfs_tag_t tag) {
return (tag & 0x40000000); return (tag & 0x40000000);
} }
static inline bool lfs_tag_hasid(lfs_tag_t tag) {
return (tag & 0x60000000) != 0x20000000;
}
static inline uint16_t lfs_tag_type(lfs_tag_t tag) { static inline uint16_t lfs_tag_type(lfs_tag_t tag) {
return (tag & 0x7fc00000) >> 22; return (tag & 0x7fc00000) >> 22;
} }
@@ -498,14 +494,14 @@ static int lfs_commit_move(lfs_t *lfs, struct lfs_commit *commit,
static int lfs_commit_commit(lfs_t *lfs, static int lfs_commit_commit(lfs_t *lfs,
struct lfs_commit *commit, lfs_mattr_t attr) { struct lfs_commit *commit, lfs_mattr_t attr) {
// filter out ids // filter out ids
if (lfs_tag_hasid(attr.tag) && ( if (lfs_tag_id(attr.tag) < 0x3ff && (
lfs_tag_id(attr.tag) < commit->filter.begin || lfs_tag_id(attr.tag) < commit->filter.begin ||
lfs_tag_id(attr.tag) >= commit->filter.end)) { lfs_tag_id(attr.tag) >= commit->filter.end)) {
return 0; return 0;
} }
// special cases // special cases
if (lfs_tag_type(attr.tag) == LFS_FROM_MOVE) { if (lfs_tag_type(attr.tag) == LFS_FROM_DIR) {
return lfs_commit_move(lfs, commit, return lfs_commit_move(lfs, commit,
lfs_tag_size(attr.tag), lfs_tag_id(attr.tag), lfs_tag_size(attr.tag), lfs_tag_id(attr.tag),
attr.u.dir, NULL); attr.u.dir, NULL);
@@ -657,8 +653,7 @@ static int lfs_commit_movescan(lfs_t *lfs, void *p, lfs_mattr_t attr) {
return 0; return 0;
} }
// TODO need this if 0x3ff is required? if (lfs_tag_id(attr.tag) != move->id.from) {
if (!lfs_tag_hasid(attr.tag) || lfs_tag_id(attr.tag) != move->id.from) {
// ignore non-matching ids // ignore non-matching ids
return 0; return 0;
} }
@@ -721,9 +716,8 @@ static int lfs_commit_globals(lfs_t *lfs, struct lfs_commit *commit,
// TODO check performance/complexity of different strategies here // TODO check performance/complexity of different strategies here
lfs_globals_t res = lfs_globals_xor(source, diff); lfs_globals_t res = lfs_globals_xor(source, diff);
int err = lfs_commit_commit(lfs, commit, (lfs_mattr_t){ int err = lfs_commit_commit(lfs, commit, (lfs_mattr_t){
lfs_mktag(LFS_TYPE_GLOBALS, lfs_mktag(LFS_TYPE_GLOBALS, 0x3ff, sizeof(res)),
res.move.id, sizeof(res.move.pair)), .u.buffer=&res});
.u.buffer=res.move.pair});
if (err) { if (err) {
return err; return err;
} }
@@ -899,15 +893,14 @@ static int lfs_dir_fetchwith(lfs_t *lfs,
return err; return err;
} }
} else if (lfs_tag_type(tag) == LFS_TYPE_GLOBALS) { } else if (lfs_tag_type(tag) == LFS_TYPE_GLOBALS) {
temp.globals.move.id = lfs_tag_id(tag);
err = lfs_bd_read(lfs, temp.pair[0], off+sizeof(tag), err = lfs_bd_read(lfs, temp.pair[0], off+sizeof(tag),
&temp.globals.move.pair, &temp.globals, sizeof(temp.globals));
sizeof(temp.globals.move.pair));
if (err) { if (err) {
return err; return err;
} }
} else { } else {
if (lfs_tag_hasid(tag) && lfs_tag_id(tag) >= temp.count) { if (lfs_tag_id(tag) < 0x3ff &&
lfs_tag_id(tag) >= temp.count) {
temp.count = lfs_tag_id(tag)+1; temp.count = lfs_tag_id(tag)+1;
} }
@@ -2822,7 +2815,7 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
err = lfs_dir_commit(lfs, &newcwd, &(lfs_mattrlist_t){ err = lfs_dir_commit(lfs, &newcwd, &(lfs_mattrlist_t){
{lfs_mktag(oldtype, newid, strlen(newpath)), {lfs_mktag(oldtype, newid, strlen(newpath)),
.u.buffer=(void*)newpath}, &(lfs_mattrlist_t){ .u.buffer=(void*)newpath}, &(lfs_mattrlist_t){
{lfs_mktag(LFS_FROM_MOVE, newid, oldid), {lfs_mktag(LFS_FROM_DIR, newid, oldid),
.u.dir=&oldcwd}}}); .u.dir=&oldcwd}}});
if (err) { if (err) {
return err; return err;

2
lfs.h
View File

@@ -118,7 +118,7 @@ enum lfs_type {
// internal chip sources // internal chip sources
LFS_FROM_REGION = 0x000, LFS_FROM_REGION = 0x000,
LFS_FROM_DISK = 0x200, LFS_FROM_DISK = 0x200,
LFS_FROM_MOVE = 0x030, LFS_FROM_DIR = 0x030,
}; };
// File open flags // File open flags

View File

@@ -59,7 +59,7 @@ tests/test.py << TEST
lfs_rename(&lfs, "b/hello", "c/hello") => 0; lfs_rename(&lfs, "b/hello", "c/hello") => 0;
lfs_unmount(&lfs) => 0; lfs_unmount(&lfs) => 0;
TEST TEST
truncate -s-11 blocks/6 truncate -s-7 blocks/6
tests/test.py << TEST tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0; lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "b") => 0; lfs_dir_open(&lfs, &dir[0], "b") => 0;
@@ -86,8 +86,8 @@ tests/test.py << TEST
lfs_rename(&lfs, "c/hello", "d/hello") => 0; lfs_rename(&lfs, "c/hello", "d/hello") => 0;
lfs_unmount(&lfs) => 0; lfs_unmount(&lfs) => 0;
TEST TEST
truncate -s-11 blocks/8 truncate -s-7 blocks/8
truncate -s-11 blocks/a truncate -s-7 blocks/a
tests/test.py << TEST tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0; lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "c") => 0; lfs_dir_open(&lfs, &dir[0], "c") => 0;
@@ -166,7 +166,7 @@ tests/test.py << TEST
lfs_rename(&lfs, "b/hi", "c/hi") => 0; lfs_rename(&lfs, "b/hi", "c/hi") => 0;
lfs_unmount(&lfs) => 0; lfs_unmount(&lfs) => 0;
TEST TEST
truncate -s-11 blocks/7 truncate -s-7 blocks/7
tests/test.py << TEST tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0; lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "b") => 0; lfs_dir_open(&lfs, &dir[0], "b") => 0;
@@ -193,8 +193,8 @@ tests/test.py << TEST
lfs_rename(&lfs, "c/hi", "d/hi") => 0; lfs_rename(&lfs, "c/hi", "d/hi") => 0;
lfs_unmount(&lfs) => 0; lfs_unmount(&lfs) => 0;
TEST TEST
truncate -s-11 blocks/9 truncate -s-7 blocks/9
truncate -s-11 blocks/b truncate -s-7 blocks/b
tests/test.py << TEST tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0; lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "c") => 0; lfs_dir_open(&lfs, &dir[0], "c") => 0;