From owner-cvs-src@FreeBSD.ORG Tue Jul 22 14:33:08 2003 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E272737B401; Tue, 22 Jul 2003 14:33:08 -0700 (PDT) Received: from phk.freebsd.dk (phk.freebsd.dk [212.242.86.175]) by mx1.FreeBSD.org (Postfix) with ESMTP id D3F2E43F75; Tue, 22 Jul 2003 14:33:07 -0700 (PDT) (envelope-from phk@phk.freebsd.dk) Received: from critter.freebsd.dk (critter.freebsd.dk [212.242.86.163]) by phk.freebsd.dk (8.12.8/8.12.8) with ESMTP id h6MLX1V3024117; Tue, 22 Jul 2003 21:33:01 GMT (envelope-from phk@phk.freebsd.dk) Received: from critter.freebsd.dk (localhost [127.0.0.1]) by critter.freebsd.dk (8.12.9/8.12.9) with ESMTP id h6MLWw5H015382; Tue, 22 Jul 2003 23:33:00 +0200 (CEST) (envelope-from phk@phk.freebsd.dk) To: "Alan L. Cox" From: "Poul-Henning Kamp" In-Reply-To: Your message of "Tue, 22 Jul 2003 15:59:37 CDT." <3F1DA5B9.A877E8D9@imimic.com> Date: Tue, 22 Jul 2003 23:32:58 +0200 Message-ID: <15381.1058909578@critter.freebsd.dk> cc: src-committers@FreeBSD.org cc: Bosko Milekic cc: Bruce Evans cc: cvs-src@FreeBSD.org cc: Steve Kargl cc: cvs-all@FreeBSD.org cc: Marcel Moolenaar Subject: Re: 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 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jul 2003 21:33:09 -0000 In message <3F1DA5B9.A877E8D9@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@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.