On Fri, Jun 25, 2010 at 9:52 AM, David Naylor <naylor.b.da...@gmail.com> wrote:
> Hi,
>
> I've created a patch that increases the performance of mtree. This is of
> particular use during a port install. In an extreme case I have experienced a
> ~20% increase [1].
>
> For a full discussion see PR bin/143732. This 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 = 0;
root = mtree_readspec(fi);
- rval = 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 = vwalk();
gcooper> This is where the performance boost is coming from as you're
not walking the directory tree, correct?
+ rval |= 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 = path;
+ fts.fts_statp = &fts_stat;
+
+ if (stat(path, fts.fts_statp))
+ return (0);
gcooper> What about symlink functionality? lstat(2)?
+ p->flags |= F_VISIT;
+ if ((p->flags & F_NOCHANGE) == 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 = 0;
gcooper> This isn't correct as per-style(9). Please do:
gcooper>
gcooper> int rval = 0;
gcooper> int serr;
gcooper>
gcooper> This reduces diff churn and is more style(9)-istically correct.
for (; p; p = p->next) {
+ if (uflag && eflag)
+ rval |= check(p, tail);
if (p->flags & F_OPT && !(p->flags & F_VISIT))
continue;
if (p->type != 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);
}
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
