Inline small files

[alexlarsson]

Some files are smaller than the size needed for the redirect and the digest, lets allow storing these inline instead.

[cgwalters]

Quick q, not a full review yet: How does this not change the digest?

[alexlarsson]

@cgwalters It changes the digest, but only if you enable it. The default change in mkcomposefs does change the default, but I was thinking we don't have a stable release yet, so it's fine to change at this point.

It doesn't for example change the digest of an ostree generated image, because it doesn't call lcfs_set_content().

[cgwalters]

Does mkfs.erofs do this inlining by default? It looks like it? Does it also use 64 bytes by default?

It feels like this is exactly the kind of thing we should worry about trying to change "post 1.0" because what if we find out that in some very important workloads it really helps to inline 128 bytes or something?

Or I guess we should aim to match mkfs.erofs, so if it has it off by default we should too...

But if we have these sorts of tunables I fear/suspect we're going to end up needing to compute multiple of them and hence allow making it a dynamic choice on the client which tunables to set like this.

[hsiangkao]

Does mkfs.erofs do this inlining by default? It looks like it? Does it also use 64 bytes by default?

Yes, mkfs.erofs inlines regular files if possible unless passing in -Enoinline_data [1]. EROFS uses 64-byte inodes just because most people need some timestamp (if a fixed timestamp is passed in like "-T0", mkfs.erofs will use 32-byte inodes instead), but 32-byte doesn't have a way to store ns timestamp (Squashfs keeps in 32-byte inodes but it's just a 4-byte timestamp without nanosecond, and it doesn't support pre-1970 time, so a bit loss on this part. But I'm not sure people really care.)

[1] erofs/erofs-utils@60549d5

[alexlarsson]

Does mkfs.erofs do this inlining by default? It looks like it? Does it also use 64 bytes by default?

I don't understand this question? mkfs.erofs will always put all file data in the filesystem image. Where else would it store it? (Although for the erofs file data, there is a question of whether to store it next to the inode, or in a separate block, but that is a different question from this PR.)

For mkcomposefs the question is whether to store the data inline in the erofs image, or whether to use a separate object file in the base directory.

Basically, suppose you have an rootfs dir with a 4 byte file, and you run mkcomposefs on it. Do we produce a erofs image that has a 4 byte file? Or do we create a erofs image that has a 0 byte file with an 65 byte overlay.redirect xattr and a 36 byte overlay.metacopy xattr, plus a 4 byte external object file?

It feels like this is exactly the kind of thing we should worry about trying to change "post 1.0" because what if we find out that in some very important workloads it really helps to inline 128 bytes or something?

There is nothing that fundamentally disallows an inlining limit of 128 bytes with the proposed API. The only place that currently hardcodes the 64 byte limit is the "convert this real directory to a lcfs-node" helper. Ostree for example doesn't use that, and could pick whatever inline limit it wanteds (if it wanted too). And, we could easily add an option to lcfs_load_node_from_file() to use a different limit if we wanted. It would be up to each user of the tools/APIs whether they wanted to enable said flag, with the result that the generated images would get a different digest.

That said, I don't think the limit is all that important once you reach some slightly larger size, because the overhead starts to be less ridiculous. So, I don't think the precise limit is all that important, as long as e.g. files smaller than a sha256 digest are inlined.

Or I guess we should aim to match mkfs.erofs, so if it has it off by default we should too...

mkfs.erofs never ever puts any files in the basedir though. The concept doesn't even exist.

But if we have these sorts of tunables I fear/suspect we're going to end up needing to compute multiple of them and hence allow making it a dynamic choice on the client which tunables to set like this.

I agree that we should limit tunables, which is why I made this the default for mkcomposefs. Maybe we should drop the "--no-inline" option though?

[giuseppe]

LGTM

[alexlarsson]

Hmm, missing here is cfs-fuse support for reading these files. Will have a look at that.

[cgwalters]

(Although for the erofs file data, there is a question of whether to store it next to the inode, or in a separate block, but that is a different question from this PR.)

Yes, I was referring to the inode.

For mkcomposefs the question is whether to store the data inline in the erofs image, or whether to use a separate object file in the base directory.

Right but I think you answered that with this:

So, I don't think the precise limit is all that important, as long as e.g. files smaller than a sha256 digest are inlined.

Yes, this makes sense to me - so I'd argue: let's do it by default, take another "checksum epoch" and try to commit to your suggested 64 bytes by default.

[cgwalters]

Just going to link this one for you and me to have a blast from the past: https://ometer.com/preferences.html - it's an argument not to add an option.

[alexlarsson]

I'm gonna merge this one and do the cfs-fuse change as part of !185 which has some other fuse stuff.

[alexlarsson]

So, I don't think the precise limit is all that important, as long as e.g. files smaller than a sha256 digest are inlined.

Yes, this makes sense to me - so I'd argue: let's do it by default, take another "checksum epoch" and try to commit to your suggested 64 bytes by default.

Well, we sort of already have a checksum epoch for the support of xattr escapes. But, I think we're now essentially ready to have a real frozen format.