Skip to content

Commit

Permalink
erofs-utils: mkfs: fix inefficient fragment deduplication
Browse files Browse the repository at this point in the history 
Currently, long fragment comparisons could cause suboptimal results,
leading to final image sizes still larger than expected:
 _________________________________________________________________________
|______ Testset _____|_______ Vanilla _________|_________ After __________| Command Line
|  CoreOS [1]        |   802107392 (765 MiB)   |   687501312 (656 MiB)    | -zlzma,6 -Eall-fragments,fragdedupe=inode -C131072
|____________________|__ 771715072 (736 MiB) __|__ 658485248 (628 MiB)  __| -zlzma,6 -Eall-fragments,fragdedupe=inode -C1048576
|  Fedora KIWI [2]   |_ 2584076288 (2465 MiB) _|_ 2550837248 (2433 MiB) __| -zlzma,6 -Eall-fragments,fragdedupe=inode -C1048576
|____________________|_ 2843598848 (2712 MiB) _|_ 2810359808 (2681 MiB) __| (Fedora-KDE-Desktop-Live-Rawhide.0.x86_64.iso)

Almost all images that use `-Eall-fragments` could benefit from this.

[1] https://builds.coreos.fedoraproject.org/prod/streams/stable/builds/41.20241215.3.0/x86_64/fedora-coreos-41.20241215.3.0-live.x86_64.iso
[2] https://pagure.io/fedora-kiwi-descriptions.git

Signed-off-by: Gao Xiang <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
@hsiangkao
hsiangkao committed Jan 21, 2025
1 parent 4cab97e commit c962911
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 70 deletions.
3 changes: 2 additions & 1 deletion lib/compress.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,8 @@ void *erofs_begin_compressed_file(struct erofs_inode *inode, int fd, u64 fpos)

if (cfg.c_fragdedupe == FRAGDEDUPE_INODE &&
inode->fragment_size < inode->i_size) {
erofs_dbg("Discard the sub-inode tail fragment @ nid %llu", inode->nid);
erofs_dbg("Discard the sub-inode tail fragment of %s",
inode->i_srcpath);
inode->fragment_size = 0;
}
}
Expand Down
125 changes: 56 additions & 69 deletions lib/fragments.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ struct erofs_fragment_dedupe_item {
u8 data[];
};

#define EROFS_FRAGMENT_INMEM_SZ_MAX EROFS_CONFIG_COMPR_MAX_SZ
#define EROFS_TOF_HASHLEN 16

#define FRAGMENT_HASHSIZE 65536
Expand All @@ -56,96 +57,82 @@ static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
struct erofs_packed_inode *epi = inode->sbi->packedinode;
struct erofs_fragment_dedupe_item *cur, *di = NULL;
struct list_head *head = &epi->hash[FRAGMENT_HASH(crc)];
unsigned int s1, e1;
erofs_off_t deduped;
u8 *data;
unsigned int length, e2, deduped;
erofs_off_t pos;
int ret;

if (list_empty(head))
return 0;

/* XXX: no need to read so much for smaller? */
if (inode->i_size < EROFS_CONFIG_COMPR_MAX_SZ)
length = inode->i_size;
else
length = EROFS_CONFIG_COMPR_MAX_SZ;

data = malloc(length);
s1 = min_t(u64, EROFS_FRAGMENT_INMEM_SZ_MAX, inode->i_size);
data = malloc(s1);
if (!data)
return -ENOMEM;

if (erofs_lseek64(fd, inode->i_size - length, SEEK_SET) < 0) {
ret = -errno;
goto out;
}

ret = read(fd, data, length);
if (ret != length) {
ret = -errno;
goto out;
ret = pread(fd, data, s1, inode->i_size - s1);
if (ret != s1) {
free(data);
return -errno;
}

DBG_BUGON(length <= EROFS_TOF_HASHLEN);
e2 = length - EROFS_TOF_HASHLEN;
e1 = s1 - EROFS_TOF_HASHLEN;
deduped = 0;

list_for_each_entry(cur, head, list) {
unsigned int e1, mn, i = 0;
unsigned int e2, mn;
erofs_off_t i, pos;

DBG_BUGON(cur->length <= EROFS_TOF_HASHLEN);
e1 = cur->length - EROFS_TOF_HASHLEN;
e2 = cur->length - EROFS_TOF_HASHLEN;

if (memcmp(cur->data + e1, data + e2, EROFS_TOF_HASHLEN))
if (memcmp(data + e1, cur->data + e2, EROFS_TOF_HASHLEN))
continue;

i = 0;
mn = min(e1, e2);
while (i < mn && cur->data[e1 - i - 1] == data[e2 - i - 1])
while (i < mn && cur->data[e2 - i - 1] == data[e1 - i - 1])
++i;

if (!di || i + EROFS_TOF_HASHLEN > deduped) {
deduped = i + EROFS_TOF_HASHLEN;
di = cur;

/* full match */
if (i == e2)
break;
i += EROFS_TOF_HASHLEN;
if (i >= s1) { /* full short match */
DBG_BUGON(i > s1);
pos = cur->pos + cur->length - s1;
while (i < inode->i_size && pos) {
char buf[2][16384];
unsigned int sz;

sz = min_t(u64, pos, sizeof(buf[0]));
sz = min_t(u64, sz, inode->i_size - i);
if (pread(fileno(epi->file), buf[0], sz,
pos - sz) != sz)
break;
if (pread(fd, buf[1], sz,
inode->i_size - i - sz) != sz)
break;

if (memcmp(buf[0], buf[1], sz))
break;
pos -= sz;
i += sz;
}
}
}
if (!di)
goto out;

DBG_BUGON(di->length < deduped);
pos = di->pos + di->length - deduped;
/* let's read more to dedupe as long as we can */
if (deduped == di->length) {
fflush(epi->file);

while(deduped < inode->i_size && pos) {
char buf[2][16384];
unsigned int sz = min_t(unsigned int, pos,
sizeof(buf[0]));

if (pread(fileno(epi->file), buf[0], sz,
pos - sz) != sz)
break;
if (pread(fd, buf[1], sz,
inode->i_size - deduped - sz) != sz)
break;

if (memcmp(buf[0], buf[1], sz))
break;
pos -= sz;
deduped += sz;
}
if (i <= deduped)
continue;
di = cur;
deduped = i;
if (deduped == inode->i_size)
break;
}
inode->fragment_size = deduped;
inode->fragmentoff = pos;

erofs_dbg("Dedupe %llu tail data at %llu", inode->fragment_size | 0ULL,
inode->fragmentoff | 0ULL);
out:
free(data);
return ret;
if (deduped) {
DBG_BUGON(!di);
inode->fragment_size = deduped;
inode->fragmentoff = di->pos + di->length - deduped;
erofs_dbg("Dedupe %llu tail data at %llu",
inode->fragment_size | 0ULL, inode->fragmentoff | 0ULL);
}
return 0;
}

int z_erofs_fragments_dedupe(struct erofs_inode *inode, int fd, u32 *tofcrc)
Expand Down Expand Up @@ -180,10 +167,10 @@ static int z_erofs_fragments_dedupe_insert(struct list_head *hash, void *data,

if (len <= EROFS_TOF_HASHLEN)
return 0;
if (len > EROFS_CONFIG_COMPR_MAX_SZ) {
data += len - EROFS_CONFIG_COMPR_MAX_SZ;
pos += len - EROFS_CONFIG_COMPR_MAX_SZ;
len = EROFS_CONFIG_COMPR_MAX_SZ;
if (len > EROFS_FRAGMENT_INMEM_SZ_MAX) {
data += len - EROFS_FRAGMENT_INMEM_SZ_MAX;
pos += len - EROFS_FRAGMENT_INMEM_SZ_MAX;
len = EROFS_FRAGMENT_INMEM_SZ_MAX;
}
di = malloc(sizeof(*di) + len);
if (!di)
Expand Down

0 comments on commit c962911

Please sign in to comment.