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

[Poul-Henning Kamp]

In message <3F1DA5B9.A877E8D9 at imimic.com>, "Alan L. Cox" writes:

>I chose my example very carefully...

There are several issues here Alan, and let us try to keep them
separated to the extent possible, the two main issues are:

1. Should GCC _always_ respect when we put __inline in the the source
   code ?

2. When should we put __inline in the source code ?

The fact that GCC silently has been ignoring __inline in the past
means that a fair number of the __inlines we have in the tree have
had no effect.

Examining the worst of the __inlines in the tree also indicates
that GCC does in fact have a much better idea about the consequence
of inlining than at least some of our programmers.  In particular
people seem to overlook recursive expansion of newbus, busdma,
VOP and KOBJ inside their inline functions.

But the fact that GCC has been ignoring the inline request is not
the same as they wouldn't have an effect (positive or negative) if
respected, it merely means that we have not by other means noticed
that they had no effect.

In other words, all inlines which GCC currently complains about
are highly suspect from a performance point of view, which also,
ipso facto means that while the heuristics of GCC may not be
optimal, it has nontheless handed us a list of suspect pieces
of source code which we should evaluate 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.

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.

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.