From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.gruenbichler@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 51C2E841F
for <pbs-devel@lists.proxmox.com>; Thu, 27 Jul 2023 10:50:56 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 3318389AE
for <pbs-devel@lists.proxmox.com>; Thu, 27 Jul 2023 10:50:26 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS
for <pbs-devel@lists.proxmox.com>; Thu, 27 Jul 2023 10:50:25 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4F5FD40C70
for <pbs-devel@lists.proxmox.com>; Thu, 27 Jul 2023 10:50:25 +0200 (CEST)
Date: Thu, 27 Jul 2023 10:50:14 +0200
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com>
References: <20230720171505.1053912-1-m.carrara@proxmox.com>
<20230720171505.1053912-2-m.carrara@proxmox.com>
In-Reply-To: <20230720171505.1053912-2-m.carrara@proxmox.com>
MIME-Version: 1.0
User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid)
Message-Id: <1690447554.6re9bwmfsv.astroid@yuna.none>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.069 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
T_SCC_BODY_TEXT_LINE -0.01 -
Subject: Re: [pbs-devel] [PATCH pxar 2/2] decoder: aio: improve performance
of async file reads
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 27 Jul 2023 08:50:56 -0000
On July 20, 2023 7:15 pm, Max Carrara wrote:
> In order to bring `aio::Decoder` on par with its `sync` counterpart
> as well as `sync::Accessor` and `aio::Accessor`, its input is now
> buffered.
>=20
> As the `tokio` docs mention themselves [0], it can be really
> inefficient to directly work with an (unbuffered) `AsyncRead`
> instance.
>=20
> The other aforementioned types already buffer their reads in one way
> or another, so wrapping the input reader in `tokio::io::BufReader`
> results in a substantial performance gain. [1]
>=20
> [0]: https://docs.rs/tokio/1.29.1/tokio/io/struct.BufReader.html
> [1]: Tested via a custom CLI utility that opens and traverses a
> large (13GB) pxar archive with each decoder and accessor
would it maybe make sense to include that CLI utility as an example
binary? it might be helpful for future work in that area, to actively
look for regressions..
> Before:
>> First pass
>> With aio::Decoder: Ok(()) (elapsed: 25.827150007s)
>> With sync::Decoder: Ok(()) (elapsed: 3.577611655s)
>> With aio::Accessor: Ok(()) (elapsed: 3.962754675s)
>> With sync::Accessor: Ok(()) (elapsed: 3.961245996s)
>>
>> Second pass
>> With aio::Decoder: Ok(()) (elapsed: 21.045064325s)
>> With sync::Decoder: Ok(()) (elapsed: 3.644003471s)
>> With aio::Accessor: Ok(()) (elapsed: 4.054085818s)
>> With sync::Accessor: Ok(()) (elapsed: 4.036097687s)
>=20
> After:
>> First pass:
>> With aio::Decoder: Ok(()) (elapsed: 7.07321221s)
>> With sync::Decoder: Ok(()) (elapsed: 3.431787191s)
>> With aio::Accessor: Ok(()) (elapsed: 3.930457465s)
>> With sync::Accessor: Ok(()) (elapsed: 4.007415416s)
>>
>> Second pass:
>> With aio::Decoder: Ok(()) (elapsed: 6.826005792s)
>> With sync::Decoder: Ok(()) (elapsed: 3.437391887s)
>> With aio::Accessor: Ok(()) (elapsed: 3.833275725s)
>> With sync::Accessor: Ok(()) (elapsed: 3.909827322s)
how were the numbers with the default buffer size of 8k? what was the
underlying storage like?
in general this seems like a good idea to me, not sure about the 16k vs
built-in default below, without more data to back it up ;)
in case you respin, IMHO patch 1 can be folded in, the patch here is
small enough and the Cargo.toml change only makes sense together with
it.. if you add the example binary, that can be a separate commit :)
> Signed-off-by: Max Carrara <m.carrara@proxmox.com>
> ---
> src/decoder/aio.rs | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>=20
> diff --git a/src/decoder/aio.rs b/src/decoder/aio.rs
> index 200dd3d..174551b 100644
> --- a/src/decoder/aio.rs
> +++ b/src/decoder/aio.rs
> @@ -79,14 +79,20 @@ mod tok {
> use std::pin::Pin;
> use std::task::{Context, Poll};
>=20
> - /// Read adapter for `futures::io::AsyncRead`
> + use tokio::io::AsyncRead;
> +
> + /// Read adapter for `tokio::io::AsyncRead`
> pub struct TokioReader<T> {
> - inner: T,
> + inner: tokio::io::BufReader<T>,
> }
>=20
> impl<T: tokio::io::AsyncRead> TokioReader<T> {
> pub fn new(inner: T) -> Self {
> - Self { inner }
> + // buffer size "sweet spot" - larger sizes don't seem to pro=
vide any benefit
> + const BUF_SIZE: usize =3D 1024 * 16;
> + Self {
> + inner: tokio::io::BufReader::with_capacity(BUF_SIZE, inn=
er),
> + }
> }
> }
>=20
> --
> 2.39.2
>=20
>=20
>=20
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>=20
>=20
>=20