[Spice-devel] [spice-common v2 12/13] quic: Remove undocumented 'no-inline' hack

[Frediano Ziglio]

> 
> The quic code goes through a function pointer in two places in order to
> try to prevent the compiler from inlining code. This does not say why
> we don't want that code to be inlined. Removing this hack even made the
> resulting object file slightly smaller (600 bytes) on my fedora 26.
> 

I agree the hack is ugly. Was documented but looking at the code
for me is pretty clear. Developers want to make sure that
write_io_word is inlined while __write_io_word is not.
Now: there are other way to specify this, both gcc and vs
have some options to specify always inline and always not
(or at least strongly suggest).
Maybe this can be achieved using SPICE_UNLIKELY.

I'm using Fedora 26 compiler too but my results seems worse,
at least looking at the size:

before
$ size common/.libs/quic.o 
   text	   data	    bss	    dec	    hex	filename
  74810	      8	  35584	 110402	  1af42	common/.libs/quic.o

after
$ size common/.libs/quic.o 
   text	   data	    bss	    dec	    hex	filename
  75122	      8	  35584	 110714	  1b07a	common/.libs/quic.o

> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  common/quic.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/common/quic.c b/common/quic.c
> index b331e09..c7884ea 100644
> --- a/common/quic.c
> +++ b/common/quic.c
> @@ -399,12 +399,10 @@ static void __write_io_word(Encoder *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
> +        __write_io_word(encoder);
>          return;
>      }
>      *(encoder->io_now++) = encoder->io_word;
> @@ -452,13 +450,10 @@ static void __read_io_word(Encoder *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
> +        __read_io_word(encoder);
>          return;
>      }
>      spice_assert(encoder->io_now < encoder->io_end);

Although I won't consider Quic code much hot code today I won't
remove an optimization without some timing considerations.

Frediano