Increase buffer size for create_regular_tmpfile_linkable_with_content #2831
Conversation
|
Hi @nanonyme. Thanks for your PR. I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
|
Hmm...I wonder if higher level code should instead be creating a https://developer-old.gnome.org/gio/stable/GBufferedInputStream.html ? What code paths were you seeing for this? What is the FUSE filesystem involved here? |
|
We're using buildbox-fuse as part of our build stack and running ostree inside it. We noticed the small reads and writes and polls result in very high overhead. It seems this is negligible with regular filesystems. Using larger buffer works consistently on both. This resulted in ballpark doubling filesystem performance. |
The small buffer size results in really bad performance under any FUSE-based filesystems with round-trips.
It's possible this could also use higher-level primitive. I was just trying to avoid larger code changes. The conversion to g_malloc was largely to avoid allocating larger amounts of memory from stack when increasing buffer size. |
|
Ah yes, so the codepath was what flatpak build-export calls on ostree. I can't recall specific call anymore but we traced the hot path to this code segment a couple of months back. |
|
FWIW, I recall reading a comment from a kernel developer a few years ago that using a classic kB sized buffer is a mistake. He suggested a much bigger buffer like 10 or even 50 MB would have much better performance with no downside. |
|
I have no problem increasing it further. The bump to 1MB is tested to improve performance and I would like a bump to at least this. It's quite trivial to change the number after switching to g_malloc, it's quite literally just a number. |
|
I think 1MB is fine. I was just acking the idea that generally we should use a bigger buffer for these read loops. |
|
/override continuous-integration/jenkins/pr-merge |
|
@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/jenkins/pr-merge In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
The small buffer size results in really bad performance under any FUSE-based filesystems with round-trips.