[Spice-devel] [PATCH spice-common] quic: Remove 'no-inline' hack

[Victor Toso]

Hi,

On Fri, May 25, 2018 at 03:12:56PM -0400, Frediano Ziglio wrote:
> >
> > Hi,
> >
> > On Fri, May 25, 2018 at 04:51:01PM +0100, Frediano Ziglio wrote:
> > > The quic code goes through a function pointer in two places in
> > > order to try to prevent the compiler from inlining code.
> > > Doing performance measurements this trick does not work anymore
> >
> > What do you mean by performance measurements? You've measured
> > with and without this patch and it is basically the same?
> >
>
> So basically that sometimes are better sometimes worst :-)
> Should I extend the comment somehow?

Well, if you have some data that you gathered, I don't think it
would hurt adding it as it mentions that the goal of this trick
is not achieved anymore.

But by all means, I'm more interested in someday be able to spot
code that try to optimize something that compiler should already
be able to do a good job (so, my question is more for my
education) ;)

> Today I merged Christophe test (sent time ago), I have a
> version that does also time measurements.

If you are used to gather data of performance, we might want to
store it somewhere for future reference too, if that makes sense.

Just thinking out loud on a Friday night ;)

Cheers,


> > > and just make code less readable.
> > 
> > Indeed, this is nicer.
> > 
> > I'm just curious about the measurement that you mention above but
> > this is an improvement from my point of view,
> > 
> > Acked-by: Victor Toso <victortoso at redhat.com>
> > 
> > > This patch and message was based on a previous work of
> > > Christophe Fergeau.
> > >
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  common/quic.c | 23 ++---------------------
> > >  1 file changed, 2 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/common/quic.c b/common/quic.c
> > > index 12e9b0b..a9a82ba 100644
> > > --- a/common/quic.c
> > > +++ b/common/quic.c
> > > @@ -388,19 +388,10 @@ static void more_io_words(Encoder *encoder)
> > >      encoder->io_end = encoder->io_now + num_io_words;
> > >  }
> > >  
> > > -static void __write_io_word(Encoder *encoder)
> > > -{
> > > -    more_io_words(encoder);
> > > -    *(encoder->io_now++) = encoder->io_word;
> > > -}
> > > -
> > > -static void (*__write_io_word_ptr)(Encoder *encoder) = __write_io_word;
> > > -
> > >  static inline void write_io_word(Encoder *encoder)
> > >  {
> > >      if (encoder->io_now == encoder->io_end) {
> > > -        __write_io_word_ptr(encoder); //disable inline optimizations
> > > -        return;
> > > +        more_io_words(encoder);
> > >      }
> > >      *(encoder->io_now++) = encoder->io_word;
> > >  }
> > > @@ -441,20 +432,10 @@ static inline void flush(Encoder *encoder)
> > >      encode(encoder, 0, 1);
> > >  }
> > >  
> > > -static void __read_io_word(Encoder *encoder)
> > > -{
> > > -    more_io_words(encoder);
> > > -    encoder->io_next_word = GUINT32_FROM_LE(*(encoder->io_now++));
> > > -}
> > > -
> > > -static void (*__read_io_word_ptr)(Encoder *encoder) = __read_io_word;
> > > -
> > > -
> > >  static inline void read_io_word(Encoder *encoder)
> > >  {
> > >      if (encoder->io_now == encoder->io_end) {
> > > -        __read_io_word_ptr(encoder); //disable inline optimizations
> > > -        return;
> > > +        more_io_words(encoder);
> > >      }
> > >      spice_assert(encoder->io_now < encoder->io_end);
> > >      encoder->io_next_word = GUINT32_FROM_LE(*(encoder->io_now++));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180525/60285270/attachment-0001.sig>