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

[Frediano Ziglio]

> 
> On Wed, Aug 02, 2017 at 05:26:47AM -0400, 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. 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.
> 
> Yes, I got that far, but *why* was it desired? Is this just a matter
> of code size? If yes, how bad was it at the time, is it better now? If
> not, what was the problem this was solving? This is what I meant by
> "undocumented"
> 
> Christophe
> 

The how bad is something you should provide in the rationale,
remove an optimization because you don't understand it and is
not documented is IMHO a bad rationale. Better to leave the
code as it.
You have to consider all the code around. write_io_word is
called only by encode, both are declared inline and encode
is called in many places. The author wanted to prevent compiler
to inline __write_io_word all these time and at the same time
help inlining encode. Beside the old way in which is implemented
it can be a big hint to the compiler. The compiler does some
statistics on code generated to inline and if is too much it
does not inline. Reducing code to inline encode (and write_io_word)
helps to keep the inlining. Considering the time this code was
written (I don't think compilers had the options to do these
declarations) it makes sense.
So the main target was making sure encode and write_io_work are
inlined (same for read path).
This affects both code size and CPU usage.

Frediano