Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lazy mode for fat32 to speed up writes #261

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Frostman
Copy link
Contributor

@Frostman Frostman commented Sep 28, 2024

For example, it reduces the time to create ~5GB file system from 7m40s to 1m40s on my laptop.

The main root cause is that currently on every space allocation Fsis/FAT are written and while they're fairly small (like 6-7MB) but it's happening on every "Write" call which is tens of sounds when writing a lot of files.

You can see on the following flame graphs that actual file writes are almost invisible comparing to writing Fsis/FAT.

With v1.4.2:

Screenshot 2024-09-27 at 8 07 00 PM

Patched, lazy mode enabled:

Screenshot 2024-09-27 at 8 20 39 PM

I tried to minimize changes to the Public API.

@Frostman Frostman marked this pull request as draft September 28, 2024 04:01
For example, it reduces time to create ~5GB file system from 7m40s
to 1m30s on my laptop.
@Frostman Frostman marked this pull request as ready for review September 28, 2024 04:47
@Frostman Frostman marked this pull request as draft September 28, 2024 04:53
@deitch
Copy link
Collaborator

deitch commented Sep 29, 2024

This is interesting. You are not saying it for everything, but as an option. Normally, every time you allocate space, you need to update the FAT tables and FSIS, so it can know what is next. That usually is the point of locking; once that is done, other things can be done somewhat more in parallel.

You are saying to have an option to only write them when I am done, by calling fs.Commit().

the time to create ~5GB file system from 7m40s to 1m40s on my laptop.

It isn't creating the 5GB filesystem, is it? It is creating the files, where each Write() allocates more space as needed? It doesn't do it on every write - I suspect most writes just write to existing space - but as soon as it runs out.

Let's think the approach through. As long as the lazy is an option - "enable this option and you run the risk of breaking things, and it no longer is thread-safe, do so at your own risk" (and I am not 100% sure it s thread-safe anyways), then I think it makes sense. However, rather than having writeFat and writeFsis return nil, I would have the parts that call them - mainly allocateSpace - choose not to call them. I would prefer if those functions were clean, every time you call writeFat, it writes the FAT. We only call those on creation of the filesystem (one time) and in allocateSpace(), so easy to control.

I also would want a dirty flag, so we know that it has not been committed.

The other question I would ask is if this is the best approach. One easy way to resolve this is to call Write() with larger chunks. If you pass it slices of 20KB, then it will call allocateSpace() far more often than if you pass it slices of 1MB. That is controllable by how you call io.Copy(), or more correctly by calling io.CopyN(). Sometimes that is not an option, but is that a better approach, than having to have an uncommitted change, and the risk that 2 parallel threads could overwrite each other?

Another option is to add fs.Truncate(path string, size in64 ). We already have that in ext4. Then you have options, all of them equally safe. For a 20MB file:

  • write it using io.Copy(), knowing it will be slower because of multiple allocations - what you do now
  • write it using io.CopyN() with a size of 1MB or 2MB, knowing it will be quicker - fast and efficient, and also least space wasteful, because it will allocate the exact size required, but in 1MB chunks until the last chunk
  • Truncate() the file to 20MB, then write in - fastest with a single allocateSpace() call, but could waste space if the file actually is 18.5MB.

Thoughts?

@Frostman
Copy link
Contributor Author

Frostman commented Sep 29, 2024

@deitch, thank you for looking into it!

Yeah, it's questionable, so I kept it as a draft to have a discussion. I agree about the dirty flag and caller-side lazy handling. I've been using io.CopyBuffer with a big buffer (IIRC 128MB) and testing on macOS, so there is no zero-copy implementation in Go, and direct writes are used. The io.CopyN uses the same code under the hood, so I'd assume it shouldn't be different. But it gives an interesting thought - why were there so many fat table writes if there was such a big buffer? I'll investigate further next week. Truncate() sounds better and should solve the problem without adding hacky APIs.

the time to create ~5GB file system from 7m40s to 1m40s on my laptop.

I'm sorry. I'm creating a filesystem from scratch and adding ~5GB of files of various sizes and a reasonably big directory tree. That's the creation of Flatcar Linux LiveCD with an Airgap installer for our product.

@deitch
Copy link
Collaborator

deitch commented Sep 30, 2024

Flatcar Linux LiveCD

Flatcar! I like not only the product, but the people behind it.

Truncate() sounds better and should solve the problem without adding hacky APIs.

Do you want to do a separate PR for that? It should be light and easy, especially because you don't need to change any other interfaces.

But it gives an interesting thought - why were there so many fat table writes if there was such a big buffer? I'll investigate further next week

I am curious. Do post what you find? Might be some bug in here, or some assumption we made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants