cvs commit: src/sys/kern init_main.c kern_malloc.c md5c.c subr_autoconf.c subr_mbuf.c subr_prf.c tty_subr.c vfs_cluster.c vfs_subr.c

[Alan L. Cox]

Poul-Henning Kamp wrote:
> 
> In message <3F1DA5B9.A877E8D9 at imimic.com>, "Alan L. Cox" writes:
> 
> >I chose my example very carefully...
> ...
> To take your example vm_object_backing_scan(), lets run it through
> the mill:
> 
> GCC has until now ignored the request to inline this function, and
> neither you, I nor anybody else noticed that  (Well, OK... Bruce
> may have noticed :-) Now GCC tells us that it ignores it, and we
> want to make an informed decision if we want to force it to be
> inlined or if GCC was right, after all.

I've known about this for several months.  :-)  (That's why I knew about
the "always inline" directive.)
 
> Your reasoning about common and constant subexpression elimination
> is a very reasonable and convincing argument for inlining it.
> 
> Compiling the file with and without the inline, and forcing GCC
> to respect the inline finds:
> 
>            text    data     bss     dec     hex filename
> inlined:  17408      76     420   17904    45f0 vm_object.o
> regular:  14944      76     420   15440    3c50 vm_object.o
>           -----
>            2464
> 
> At least I find that 2k+ code is a non-trivial amount which is
> likely, through prefetch and cache flushing, to have a negative
> performance impact.
> 
> Given two conflicting arguments, we need to resort to reality for
> the answer, and what we need to reinstate the inline on an informed
> basis is a realistic benchmark which indicates a positive performance
> impact of the (respected) inline request.
> 
> Until we have that benchmark, the inline has been removed because
> that retains the status quo and puts us closer to being able to get
> the kernel -Werror again (by raising the inline-limit) without
> bloating the kernel text segment with 100k+ produced by inlines
> which may not be beneficial.
> 

In general, I agree with you that a vast majority of the inlines should
be removed.  We should, however, have an override for exceptional
cases.  Again, in the case of vm_object_backing_scan(), code size is a
bad predictor of inlining's effect.  Inlining is being used to achieve a
form of code specialization that will actually reduce the size of the
code that is *executed*.

In conclusion, my point is not that you should stop what you're doing. 
It is rather that there are exceptional cases where gcc is doing the
wrong thing and we should have an override to force inlining that can be
applied.

Alan