[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/2] QIOChannelSocket: Reduce ifdefs to improve readabilit
From: |
Leonardo Bras Soares Passos |
Subject: |
Re: [PATCH v2 1/2] QIOChannelSocket: Reduce ifdefs to improve readability |
Date: |
Mon, 13 Jun 2022 18:21:18 -0300 |
On Fri, Jun 10, 2022 at 5:25 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
[...]
> Ok, so if it is checked earlier then we merely need an assert.
>
> if (flags & QIO_CHANNEL_WRITE_FLAG_ZERO_COPY) {
> #ifdef QEMU_MSG_ZEROCOPY
> sflags = MSG_ZEROCOPY;
> zero_copy_enabled = true;
> #else
> g_assert_unreachable();
> #endif
> > }
Ok, I will add that in the next version.
>
>
>
> > > > @@ -592,15 +594,13 @@ static ssize_t
> > > > qio_channel_socket_writev(QIOChannel *ioc,
> > > > return QIO_CHANNEL_ERR_BLOCK;
> > > > case EINTR:
> > > > goto retry;
> > > > -#ifdef QEMU_MSG_ZEROCOPY
> > > > case ENOBUFS:
> > > > - if (sflags & MSG_ZEROCOPY) {
> > > > + if (zero_copy_enabled) {
> > >
> > > if (flags & QIO_CHANNEL_WRITE_FLAG_ZERO_COPY)
> > >
> > > avoids the #ifdef without needing to add yet another
> > > variable expressing what's already expressed in both
> > > 'flags' and 'sflags'.
> >
> > Yes, it does, but at the cost of not compiling-out the zero-copy part
> > when it's not supported,
> > since the QIO_CHANNEL_WRITE_FLAG_ZERO_COPY comes as a parameter. This ends
> > up
> > meaning there will be at least one extra test for every time this
> > function is called (the one in the next patch).
>
> The cost of a simple bit test is between negligible-and-non-existant
> with branch prediction. I doubt it would be possible to even measure
> it.
Yeah, you are probably right on that.
So the main learning point here is that it's not worth creating a new
boolean for compiling-out
code that should not impact performance ?
I mean, if performance-wise they should be the same, then a new
variable would be just a
bother for the programmer.
Best regards,
Leo
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>