Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 25 Jun 2010 11:01:42 -0700
From:      Garrett Cooper <yanefbsd@gmail.com>
To:        David Naylor <naylor.b.david@gmail.com>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: [RFC] mtree improvements
Message-ID:  <AANLkTilWRu63O48AotRODxnwo8EtnSHB27UucXzSJDf9@mail.gmail.com>
In-Reply-To: <201006251852.30302.naylor.b.david@gmail.com>
References:  <201006251852.30302.naylor.b.david@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jun 25, 2010 at 9:52 AM, David Naylor <naylor.b.david@gmail.com> 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);
 }



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