From owner-freebsd-hackers@FreeBSD.ORG Fri Jun 25 18:01:43 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 64F5F1065692 for ; Fri, 25 Jun 2010 18:01:43 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from mail-gy0-f182.google.com (mail-gy0-f182.google.com [209.85.160.182]) by mx1.freebsd.org (Postfix) with ESMTP id 210388FC19 for ; Fri, 25 Jun 2010 18:01:42 +0000 (UTC) Received: by gyh20 with SMTP id 20so6567238gyh.13 for ; Fri, 25 Jun 2010 11:01:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=FpSlM0vefHp94Z1cl2SdH1FWogKLwPE4yNaR4I1i67w=; b=pntZKILe2VC51GbBDIb5mXUqt8568B//nxkscoxWcSBNlOIv6J/RPgi8ptGNKv8PfA YVeIWUVB9bfIzXJskIAVHQI1EzPEFN9R/CcMZZ9Id8ttdm+UoAOTFBdgHICei8jA7lY2 5H5evOweQ/VHcMwezY8u9wpTiYh6gprEA6QAw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=I4MkdWH9KoQatL3EAVPDeFjENTNOMc6zSdBiGQCjZ4XkqhTr64mjzLrBe5yiu42ffj YPQJgZsg4qC6I1MzCi5TzM9+nlE9RwmFzLayufQYPn9dKYAeNQ6DpUP0KnZVsV3R+Tp+ 0xn96ZqeZJrCFquSwPVmcj+ALHEmrvxtuMabE= MIME-Version: 1.0 Received: by 10.229.181.142 with SMTP id by14mr744117qcb.18.1277488902209; Fri, 25 Jun 2010 11:01:42 -0700 (PDT) Received: by 10.229.80.75 with HTTP; Fri, 25 Jun 2010 11:01:42 -0700 (PDT) In-Reply-To: <201006251852.30302.naylor.b.david@gmail.com> References: <201006251852.30302.naylor.b.david@gmail.com> Date: Fri, 25 Jun 2010 11:01:42 -0700 Message-ID: From: Garrett Cooper To: David Naylor Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org Subject: Re: [RFC] mtree improvements X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Jun 2010 18:01:43 -0000 On Fri, Jun 25, 2010 at 9:52 AM, David Naylor wr= ote: > Hi, > > I've created a patch that increases the performance of mtree. =A0This is = of > particular use during a port install. =A0In an extreme case I have experi= enced a > ~20% increase [1]. > > For a full discussion see PR bin/143732. =A0This arose out of [2] where I > experienced the increase. > > For your convenience I have attached the patch. > > Please review this patch and if it is acceptable, commit it. > > Regards, > > David > > 1] http://markmail.org/message/iju3l6hyv7s7emrb > 2] http://markmail.org/message/gfztjpszl5dozzii Hmm... this has other interesting applications other than just ports, but unfortunately pkg_install won't really feel as much of a performance boost (because it uses mtree -e -U when +MTREE exists in the package). Comments follow. Thanks! -Garrett --- /usr/src/usr.sbin/mtree/verify.c 2010-02-07 15:07:28.000000000 +0200 +++ verify.c 2010-02-07 15:04:10.000000000 +0200 @@ -50,17 +50,23 @@ static NODE *root; static char path[MAXPATHLEN]; -static void miss(NODE *, char *); +static int miss(NODE *, char *); +static int check(NODE *, char *); static int vwalk(void); int mtree_verifyspec(FILE *fi) { - int rval; + int rval =3D 0; root =3D mtree_readspec(fi); - rval =3D vwalk(); - miss(root, path); + /* + * No need to walk entire tree if we are only updating the structure + * and extra files are ignored. + */ + if (!(uflag && eflag)) + rval =3D vwalk(); gcooper> This is where the performance boost is coming from as you're not walking the directory tree, correct? + rval |=3D miss(root, path); return (rval); } @@ -155,15 +161,47 @@ return (rval); } -static void +static int +check(NODE *p, char *tail) +{ + FTSENT fts; + struct stat fts_stat; + + strcpy(tail, p->name); gcooper> Dangerous. Please use strlcpy with appropriate bounds. + /* + * It is assumed that compare() only requires fts_accpath and fts_statp + * fields in the FTSENT structure. + */ + fts.fts_accpath =3D path; + fts.fts_statp =3D &fts_stat; + + if (stat(path, fts.fts_statp)) + return (0); gcooper> What about symlink functionality? lstat(2)? + p->flags |=3D F_VISIT; + if ((p->flags & F_NOCHANGE) =3D=3D 0 && compare(p->name, p, &fts)) + return (MISMATCHEXIT); + else + return (0); + + /* + * tail is not restored to '\0' as the next time tail (or path) is used + * is with a strcpy (thus overriding the '\0'). See +19 lines below. + */ +} + +static int miss(NODE *p, char *tail) { int create; char *tp; const char *type, *what; - int serr; + int serr, rval =3D 0; gcooper> This isn't correct as per-style(9). Please do: gcooper> gcooper> int rval =3D 0; gcooper> int serr; gcooper> gcooper> This reduces diff churn and is more style(9)-istically correct. for (; p; p =3D p->next) { + if (uflag && eflag) + rval |=3D check(p, tail); if (p->flags & F_OPT && !(p->flags & F_VISIT)) continue; if (p->type !=3D F_DIR && (dflag || p->flags & F_VISIT)) @@ -256,4 +294,5 @@ (void)printf("%s: file flags not set: %s\n", path, strerror(errno)); } + return (rval); }