From owner-svn-src-all@FreeBSD.ORG Sat Jul 12 11:34:32 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2144DAE6; Sat, 12 Jul 2014 11:34:32 +0000 (UTC) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id C11B32A98; Sat, 12 Jul 2014 11:34:31 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 96C6A780AC2; Sat, 12 Jul 2014 21:34:29 +1000 (EST) Date: Sat, 12 Jul 2014 21:34:28 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: David Chisnall Subject: Re: svn commit: r268566 - head/usr.bin/users In-Reply-To: <201407120747.s6C7lpYE020200@svn.freebsd.org> Message-ID: <20140712195452.N3279@besplex.bde.org> References: <201407120747.s6C7lpYE020200@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=eojmkOZX c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=-KJylB5gLocA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=AH46BtWyvaMZlGIRI_EA:9 a=b8kHId1VmKFv8_xZ:21 a=s2aPjgIK9Rb9x3bt:21 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Jul 2014 11:34:32 -0000 On Sat, 12 Jul 2014, David Chisnall wrote: > Log: > Turn off exceptions and rtti when building the c++ version of users. > Neither is used in the program and this saves us 10KB (around 40%) in binary > size. This joke is bad. The C++ version is worse in every way. Source code: It becomes smaller mainly by removing the removing the formal use of getopt() and usage() and the error checking for allocation failure. (Error checking for setutxent(), getutxent(), strlcpy() and printf() was already missing (not supported by the API for the first 2).) Checking for errors and consistent error handling are not really necessary for such a small unimportant program. The difference from not using getopt() and usage() of shows up as unusual diagnostics for usage errors. Old "users -fubar" prints "users: illegal option -- f\nusage: users\n". New "users -fubar" prints "usage: users\n". Most programs use getopt() and usage() similarly to get consistent error handling. However, there is a nice consistency bug in this: if you rename old users to oldusers, then the error message for "oldusers -fubar" is "oldusers: illegal...usage: users...". This is because the first line of the error message is printed from deep in getopt() and it uses __getprogname() for the name, while the usage message hard-codes the normal program name as required by style(9). The program was so trivial and the getopt()+usage() code so badlly written that the latter comprised almost 25% of the program not counting the copyright (almost 20 lines out of 80). If it were better written it could be reduced to only 15% of the program. In the C++ version, it is reduced to 12.5% of the program (5 lines out of 40). Allocation failure is not really worth checking for, especially in such a small unimportant program. On freefall now, there are 18 users. Some systems have more users, but most have less. I tried checking for the error mishandling using ulimit -Sd, but malloc() is too broken to support the datasize ulimit so I couldn't get any interesting failures. I tried putting 1 million dummy users in the array. ulimit -Sd is even more broken for dynamic linkage. Dynamically linked users didn't fail until the datasize limit was reduced to 3K. It ran using a few MB of RSS. Statically linked users failed when the datasize limit was reduced to 67k. The failure modes were uninteresting -- just an abort trap before main() is reached. Once main() was reached, the broken malloc() allowed 1GB of allocations without further traps. Testing in another program showed the expected brokenness. The C program with error checking exited gracefully. The C++ program with null error checking aborted with core dump. The density of the allocation was also bad for the C++ program. The C++ program allocated 1806 objects of size 1MB each using malloc() before failing. The C++ program failed after only 1024 allocations of size 1MB each using names.push_back(). Both (after removing the error checking from the C program) generated a core file of size 2.3GB. 2.3GB takes about 70 seconds to dump on ref11-i386. Compile time: New users takes 1.63 seconds to compile on freefall (static linkage; dynamic is not quite so bad). Anything more than 10 milliseconds is slow. Old users takes 180 milliseconds to compile on freefall and 50 milliseconds on my local machine. My local machine uses 1/24 as many CPUs each running 1/2 as fast for this, on slower disks, but is missing other handicaps. Program size: text data bss dec hex filename 349578 13196 54200 416974 65cce users-c-static 897134 8940 59181 965255 eba87 users-ex-static 893410 8940 59181 961531 eabfb users-noex-static 2915 628 16 3559 de7 users-c-dynamic 19570 804 416 20790 5136 users-ex-dynamic 16282 708 344 17334 43b6 users-noex-dynamic users-c is the old C users. users-ex is the C++ users before this commit. users-noex is the C++ users with this commit. I don't see any 40% reduction. For the statically linked case, the reduction of 3.7K is in the noise compared with 540K of bloat relative to the C version. Anything more than 16K for the statically linked program is bloated. Run time: C++ sort() was twice as slow as qsort() for sorting 1 million dummy users in the allocation tests. About 8 seconds instead of 4, except when compiled with -g -O0 it was 15 seconds instead of 4. Bruce