Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 12 Jul 2014 21:34:28 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        David Chisnall <theraven@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r268566 - head/usr.bin/users
Message-ID:  <20140712195452.N3279@besplex.bde.org>
In-Reply-To: <201407120747.s6C7lpYE020200@svn.freebsd.org>
References:  <201407120747.s6C7lpYE020200@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140712195452.N3279>