Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 21 Mar 2002 08:24:04 +1100 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        Ruslan Ermilov <ru@FreeBSD.org>
Cc:        Dag-Erling Smorgrav <des@FreeBSD.org>, Bruce Evans <bde@FreeBSD.org>, <current@FreeBSD.org>
Subject:   Re: cvs commit: src/usr.bin/xinstall xinstall.c
Message-ID:  <20020321075804.A11453-100000@gamplex.bde.org>
In-Reply-To: <20020320093359.GB2503@sunbay.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 20 Mar 2002, Ruslan Ermilov wrote:

> On Mon, Mar 18, 2002 at 03:26:13PM -0800, Dag-Erling Smorgrav wrote:
> > des         2002/03/18 15:26:13 PST
> >
> >   Modified files:
> >     usr.bin/xinstall     xinstall.c
> >   Log:
> >   Bump the cutoff mark for comparing files from 8 MB to 16 MB.
> >
> >   Revision  Changes    Path
> >   1.48      +3 -1      src/usr.bin/xinstall/xinstall.c
> >
> OK, I think it's finally the time to borrow mmap(2) comparison in
> chunks from OpenBSD (better viewed as -w diff):

Using mmap(2) at all is over-engineered IMO.  Does it make even 1 1%
difference for heavy uses like installworld?  I get the following times
on an Athlon1600 for installing an 8MB file 16 times after it has
already been installed and caches warmed up:

install -C:    0.48 real         0.38 user         0.09 sys
install -MC:   0.86 real         0.26 user         0.59 sys

and for installing a copy of /usr/bin/*:

install -C:    0.29 real         0.24 user         0.04 sys
install -MC:   0.54 real         0.10 user         0.42 sys

Using mmap(2) in cmp(1) makes more difference because the non-mmap
case of cmp(1) is poorly implemented.  cmp(1) uses a getc() loop, but
install(1) reads MAXBSIZE at a time).

> %%%
> Index: xinstall.c
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/xinstall/xinstall.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 xinstall.c
> --- xinstall.c	18 Mar 2002 23:26:13 -0000	1.48
> +++ xinstall.c	20 Mar 2002 09:26:11 -0000
> @@ -74,12 +74,11 @@ static const char sccsid[] = "From: @(#)
> ...
> +out:
> +	if (!done_compare && from_len <= MAX_CMP_SIZE) {
> +		char buf1[MAXBSIZE];
> +		char buf2[MAXBSIZE];
> +		int n1, n2;

This is cleaner than before, but still has style bugs (nested declarations),
and still attempts to pessimize the !mmap case by using misaligned buffers
(copy() is missing the style bugs but not the misaligment attempt).

> ...
>  	} else
>  		rv = 1;	/* don't bother in this case */

We should bother now, to give the documented semantics for -C (remove the
"from_len <= MAX_CMP_SIZE" check above).

Bruce


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




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