[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] coroutine: resize pool periodically instead of limiting size
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] coroutine: resize pool periodically instead of limiting size |
Date: |
Thu, 2 Sep 2021 12:24:34 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 9/1/21 6:09 PM, Stefan Hajnoczi wrote:
> It was reported that enabling SafeStack reduces IOPS significantly
> (>25%) with the following fio benchmark on virtio-blk using a NVMe host
> block device:
>
> # fio --rw=randrw --bs=4k --iodepth=64 --runtime=1m --direct=1 \
> --filename=/dev/vdb --name=job1 --ioengine=libaio --thread \
> --group_reporting --numjobs=16 --time_based \
> --output=/tmp/fio_result
>
> Serge Guelton and I found that SafeStack is not really at fault, it just
> increases the cost of coroutine creation. This fio workload exhausts the
> coroutine pool and coroutine creation becomes a bottleneck. Previous
> work by Honghao Wang also pointed to excessive coroutine creation.
>
> Creating new coroutines is expensive due to allocating new stacks with
> mmap(2) and mprotect(2). Currently there are thread-local and global
> pools that recycle old Coroutine objects and their stacks but the
> hardcoded size limit of 64 for thread-local pools and 128 for the global
> pool is insufficient for the fio benchmark shown above.
>
> This patch changes the coroutine pool algorithm to a simple thread-local
> pool without a size limit. Threads periodically shrink the pool down to
> a size sufficient for the maximum observed number of coroutines.
>
> This is a very simple algorithm. Fancier things could be done like
> keeping a minimum number of coroutines around to avoid latency when a
> new coroutine is created after a long period of inactivity. Another
> thought is to stop the timer when the pool size is zero for power saving
> on threads that aren't using coroutines. However, I'd rather not add
> bells and whistles unless they are really necessary.
>
> The global pool is removed by this patch. It can help to hide the fact
> that local pools are easily exhausted, but it's doesn't fix the root
> cause. I don't think there is a need for a global pool because QEMU's
> threads are long-lived, so let's keep things simple.
>
> Performance of the above fio benchmark is as follows:
>
> Before After
> IOPS 60k 97k
>
> Memory usage varies over time as needed by the workload:
>
> VSZ (KB) RSS (KB)
> Before fio 4705248 843128
> During fio 5747668 (+ ~100 MB) 849280
> After fio 4694996 (- ~100 MB) 845184
>
> This confirms that coroutines are indeed being freed when no longer
> needed.
>
> Thanks to Serge Guelton for working on identifying the bottleneck with
> me!
>
> Reported-by: Tingting Mao <timao@redhat.com>
> Cc: Serge Guelton <sguelton@redhat.com>
> Cc: Honghao Wang <wanghonghao@bytedance.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Daniele Buono <dbuono@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/qemu/coroutine-pool-timer.h | 36 +++++++++++++++++
> include/qemu/coroutine.h | 7 ++++
> iothread.c | 6 +++
> util/coroutine-pool-timer.c | 35 ++++++++++++++++
> util/main-loop.c | 5 +++
> util/qemu-coroutine.c | 62 ++++++++++++++---------------
> util/meson.build | 1 +
> 7 files changed, 119 insertions(+), 33 deletions(-)
> create mode 100644 include/qemu/coroutine-pool-timer.h
> create mode 100644 util/coroutine-pool-timer.c
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>