From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pve-devel] applied: [PATCH v2 common] tools: file_set_contents: use syswrite instead of print
Date: Mon, 14 Oct 2024 11:07:45 +0200 [thread overview]
Message-ID: <a4c5e7ba-f85c-4ed6-b25e-38851d7a476e@proxmox.com> (raw)
In-Reply-To: <e4a78a89-9b1d-48e0-8609-556752457ac4@proxmox.com>
On 10/14/24 10:42, Thomas Lamprecht wrote:
> Am 30/09/2024 um 13:40 schrieb Filip Schauer:
>> The use of `print` can be inefficient for writing larger files due to
>> its default buffering in 8 KiB blocks.
>>
>> This is especially problematic on `pmxcfs` where files are written in
>> 4 KiB blocks due to the defaults of `libfuse2`. This leads to
>> significant write amplification on files larger than 4 KiB.
>>
>> Patch (fix #5728: pmxcfs: allow bigger writes than 4k for fuse) [1]
>> addresses this by enabling `big_writes`, allowing up to 128 KiB blocks.
>> But due to the use of `print` in `file_set_contents`, writes are still
>> only buffered in 8 KiB blocks.
>>
>> To further address this, this commit switches to using `syswrite`
>> instead of `print` to mitigate the block size limit imposed by `print`.
>> Combined with patch [1], file writes to `/etc/pve/` are now buffered in
>> 128 KiB blocks.
>>
>> The table below illustrates the drastic reduction in write
>> amplification when writing files of different sizes to `/etc/pve/` using
>> `file_set_contents`:
>>
>> print big_writes+print big_writes+syswrite
>> file size written amplif. written amplif. written amplif.
>> 1 KiB 48 KiB 48.0 45 KiB 45.0 41 KiB 41.0
>> 2 KiB 48 KiB 24.0 45 KiB 22.5 62 KiB 31.0
>> 4 KiB 82 KiB 20.5 80 KiB 20.0 73 KiB 18.3
>> 8 KiB 121 KiB 15.1 90 KiB 11.3 89 KiB 11.1
>> 16 KiB 217 KiB 13.6 146 KiB 9.1 113 KiB 7.1
>> 32 KiB 506 KiB 15.8 314 KiB 9.8 158 KiB 4.9
>> 64 KiB 1472 KiB 23.0 826 KiB 12.9 259 KiB 4.0
>> 128 KiB 5585 KiB 43.6 3765 KiB 29.4 452 KiB 3.5
>> 256 KiB 20424 KiB 79.8 10743 KiB 42.0 2351 KiB 9.2
>> 512 KiB 86715 KiB 169.4 43650 KiB 85.3 3204 KiB 6.3
>> 1024 KiB 369568 KiB 360.9 187496 KiB 183.1 15845 KiB 15.5
>>
>> Since `file_set_contents` also performs a `rename` after writing, the
>> following table shows the results when the file is written without
>> renaming it afterwards:
>>
>> print big_writes+print big_writes+syswrite
>> file size written amplif. written amplif. written amplif.
>> 1 KiB 29 KiB 29.0 29 KiB 29.0 25 KiB 25.0
>> 2 KiB 29 KiB 14.5 30 KiB 15.0 25 KiB 12.5
>> 4 KiB 37 KiB 9.3 44 KiB 11.0 41 KiB 10.3
>> 8 KiB 61 KiB 7.6 45 KiB 5.6 45 KiB 5.6
>> 16 KiB 143 KiB 8.9 86 KiB 5.4 57 KiB 3.6
>> 32 KiB 396 KiB 12.4 225 KiB 7.0 69 KiB 2.2
>> 64 KiB 1281 KiB 20.0 673 KiB 10.5 105 KiB 1.6
>> 128 KiB 4789 KiB 37.4 3478 KiB 27.2 169 KiB 1.3
>> 256 KiB 18868 KiB 73.7 9976 KiB 39.0 572 KiB 2.2
>> 512 KiB 79304 KiB 154.9 42714 KiB 83.4 2150 KiB 4.2
>> 1024 KiB 347929 KiB 339.8 182483 KiB 178.2 11133 KiB 10.9
>>
>> [1] https://lists.proxmox.com/pipermail/pve-devel/2024-September/065396.html
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>> Changes since v1:
>> * Add benchmark results without rename to commit message
>> * Fix "Wide character in syswrite" error by first encoding $data with print
>>
>> src/PVE/Tools.pm | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>>
>
> applied, and many thanks for the detailed benchmarks!
>
hi since you applied this, we probably also want to apply the pmxcfs patch?
(since without that, this patch does not really have an effect)
should i send a new version (as non rfc) with a better commit message for that
or is that not necessary from your POV ?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2024-10-14 9:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-30 11:40 [pve-devel] " Filip Schauer
2024-10-14 8:42 ` [pve-devel] applied: " Thomas Lamprecht
2024-10-14 9:07 ` Dominik Csapak [this message]
2024-10-14 9:22 ` Thomas Lamprecht
2024-10-14 9:26 ` [pve-devel] " Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a4c5e7ba-f85c-4ed6-b25e-38851d7a476e@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox