[PATCH] update: making "test" an ash built-in

Bernhard Fischer rep.nop at aon.at
Thu Jun 8 13:06:02 UTC 2006


On Thu, Jun 08, 2006 at 01:54:16PM +0200, Natanael Copa wrote:
>On Wed, 2006-06-07 at 17:04 -0400, Paul Fox wrote:
>> hi --
>> 
>> the help text for CONFIG_TEST currently says that "test" is a
>> builtin command in ash.  but it's not, and on slower systems the
>> performance hit is noticeable.
>
>Even on my Pentium D the performance hit was so huge that busybox ash
>was completely unuseable.
>
>> it seems to work, and running test in a tight shell loop is
>> between 75 and 150 times faster (depending on whether the test
>> conditions are accessing the filesystem or not).  this is on
>> a slow ARM with little cache.  better machines will show less
>> improvement, i'm sure.
>
>The difference was big even on faster computers.
>
>> any objections if i commit this?
>
>You need builtins for '[' and '[[' too. Please see the attatched update
>patch.
>
>Its agains current svn.
>
>The patch below is just for an overview what differs the old
>coreutils/test.c with the new libbb/bb_test.c.
>
>Note that I replaced the rest of the bb_error_msg_and_die with only
>bb_error_msg. You would not like the entire shell die just because you
>entered wrong number of params to 'test', '[' or '[['.
>
>I would love to see it committed.

Could you guys please merge that attached bb.test.01b.diff too, while at
it? It saves about 55B for me and slows down the applet only marginally
(like all case-unrolling which saves space). Also note the commented out
"else" in the patch. From the looks the else there should be ok (and
would save another 15 bytes or so).

I'm also attaching a patch against current -svn which is n updated patch
i did last year to see how long it takes to run an applet.
E.g.:
$ ./test --time 1 -le 12000
Took 8773 ticks
$ ./test --time=1588.557 1 -le 12000
Took 5.65104 µsec
(that box's CPU runs at 1588MHz).

I'm planning on looking into cleaning up the "--help" text parsing,
since it's just not clean to strcmp for it in _two_ places.
-------------- next part --------------
Index: coreutils/test.c
===================================================================
--- coreutils/test.c	(revision 15337)
+++ coreutils/test.c	(working copy)
@@ -13,31 +13,19 @@
  *	modified by Erik Andersen <andersen at codepoet.org> to be used
  *	in busybox.
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
+ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
  *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
  * Original copyright notice states:
  *	"This program is in the Public Domain."
  */
 
+#include "busybox.h"
 #include <sys/types.h>
 #include <unistd.h>
 #include <ctype.h>
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
-#include "busybox.h"
 
 /* test(1) accepts the following grammar:
 	oexpr	::= aexpr | aexpr "-o" oexpr ;
@@ -108,8 +96,8 @@
 };
 
 static const struct t_op {
-	const char *op_text;
-	short op_num, op_type;
+	const char * const op_text;
+	const short op_num, op_type;
 } ops[] = {
 	{
 	"-r", FILRD, UNOP}, {
@@ -167,71 +155,43 @@
 static gid_t *group_array = NULL;
 static int ngroups;
 
-static enum token t_lex(char *s);
-static arith_t oexpr(enum token n);
-static arith_t aexpr(enum token n);
-static arith_t nexpr(enum token n);
-static int binop(void);
 static arith_t primary(enum token n);
-static int filstat(char *nm, enum token mode);
-static arith_t getn(const char *s);
-static int newerf(const char *f1, const char *f2);
-static int olderf(const char *f1, const char *f2);
-static int equalf(const char *f1, const char *f2);
-static int test_eaccess(char *path, int mode);
-static int is_a_group_member(gid_t gid);
-static void initialize_group_array(void);
 
-int test_main(int argc, char **argv)
+static enum token t_lex(char *s)
 {
-	int res;
+	struct t_op const *op = ops;
 
-	if (strcmp(bb_applet_name, "[") == 0) {
-		if (strcmp(argv[--argc], "]"))
-			bb_error_msg_and_die("missing ]");
-		argv[argc] = NULL;
+	if (s == 0) {
+		t_wp_op = (struct t_op *) 0;
+		return EOI;
 	}
-	if (strcmp(bb_applet_name, "[[") == 0) {
-		if (strcmp(argv[--argc], "]]"))
-			bb_error_msg_and_die("missing ]]");
-		argv[argc] = NULL;
-	}
-	/* Implement special cases from POSIX.2, section 4.62.4 */
-	switch (argc) {
-	case 1:
-		exit(1);
-	case 2:
-		exit(*argv[1] == '\0');
-	case 3:
-		if (argv[1][0] == '!' && argv[1][1] == '\0') {
-			exit(!(*argv[2] == '\0'));
+	while (op->op_text) {
+		if (strcmp(s, op->op_text) == 0) {
+			t_wp_op = op;
+			return op->op_num;
 		}
-		break;
-	case 4:
-		if (argv[1][0] != '!' || argv[1][1] != '\0') {
-			if (t_lex(argv[2]), t_wp_op && t_wp_op->op_type == BINOP) {
-				t_wp = &argv[1];
-				exit(binop() == 0);
-			}
-		}
-		break;
-	case 5:
-		if (argv[1][0] == '!' && argv[1][1] == '\0') {
-			if (t_lex(argv[3]), t_wp_op && t_wp_op->op_type == BINOP) {
-				t_wp = &argv[2];
-				exit(!(binop() == 0));
-			}
-		}
-		break;
+		op++;
 	}
+	t_wp_op = (struct t_op *) 0;
+	return OPERAND;
+}
 
-	t_wp = &argv[1];
-	res = !oexpr(t_lex(*t_wp));
+static arith_t nexpr(enum token n)
+{
+	if (n == UNOT)
+		return !nexpr(t_lex(*++t_wp));
+	return primary(n);
+}
 
-	if (*t_wp != NULL && *++t_wp != NULL)
-		bb_error_msg_and_die("%s: unknown operand", *t_wp);
+static arith_t aexpr(enum token n)
+{
+	arith_t res;
 
-	return (res);
+	res = nexpr(n);
+	if (t_lex(*++t_wp) == BAND)
+		return aexpr(t_lex(*++t_wp)) && res;
+	t_wp--;
+	return res;
 }
 
 static arith_t oexpr(enum token n)
@@ -246,102 +206,91 @@
 	return res;
 }
 
-static arith_t aexpr(enum token n)
+/* atoi with error detection */
+static arith_t getn(const char *s)
 {
-	arith_t res;
+	char *p;
+#ifdef CONFIG_FEATURE_TEST_64
+	long long r;
+#else
+	long r;
+#endif
 
-	res = nexpr(n);
-	if (t_lex(*++t_wp) == BAND)
-		return aexpr(t_lex(*++t_wp)) && res;
-	t_wp--;
-	return res;
+	errno = 0;
+#ifdef CONFIG_FEATURE_TEST_64
+	r = strtoll(s, &p, 10);
+#else
+	r = strtol(s, &p, 10);
+#endif
+
+	if (errno != 0)
+		bb_error_msg_and_die("%s: out of range", s);
+
+	/*   p = bb_skip_whitespace(p); avoid const warning */
+	if (*(bb_skip_whitespace(p)))
+		bb_error_msg_and_die("%s: bad number", s);
+
+	return r;
 }
 
-static arith_t nexpr(enum token n)
+static void initialize_group_array(void)
 {
-	if (n == UNOT)
-		return !nexpr(t_lex(*++t_wp));
-	return primary(n);
+	ngroups = getgroups(0, NULL);
+	group_array = xrealloc(group_array, ngroups * sizeof(gid_t));
+	getgroups(ngroups, group_array);
 }
 
-static arith_t primary(enum token n)
+/* Return non-zero if GID is one that we have in our groups list. */
+static int is_a_group_member(gid_t gid)
 {
-	arith_t res;
+	register int i;
 
-	if (n == EOI) {
-		bb_error_msg_and_die("argument expected");
-	}
-	if (n == LPAREN) {
-		res = oexpr(t_lex(*++t_wp));
-		if (t_lex(*++t_wp) != RPAREN)
-			bb_error_msg_and_die("closing paren expected");
-		return res;
-	}
-	if (t_wp_op && t_wp_op->op_type == UNOP) {
-		/* unary expression */
-		if (*++t_wp == NULL)
-			bb_error_msg_and_die(bb_msg_requires_arg, t_wp_op->op_text);
-		switch (n) {
-		case STREZ:
-			return strlen(*t_wp) == 0;
-		case STRNZ:
-			return strlen(*t_wp) != 0;
-		case FILTT:
-			return isatty(getn(*t_wp));
-		default:
-			return filstat(*t_wp, n);
-		}
-	}
+	/* Short-circuit if possible, maybe saving a call to getgroups(). */
+	if (gid == getgid() || gid == getegid())
+		return (1);
 
-	if (t_lex(t_wp[1]), t_wp_op && t_wp_op->op_type == BINOP) {
-		return binop();
-	}
+	if (ngroups == 0)
+		initialize_group_array();
 
-	return strlen(*t_wp) > 0;
+	/* Search through the list looking for GID. */
+	for (i = 0; i < ngroups; i++)
+		if (gid == group_array[i])
+			return (1);
+
+	return (0);
 }
 
-static int binop(void)
+/* Do the same thing access(2) does, but use the effective uid and gid,
+   and don't make the mistake of telling root that any file is
+   executable. */
+static int test_eaccess(char *path, int mode)
 {
-	const char *opnd1, *opnd2;
-	struct t_op const *op;
+	struct stat st;
+	unsigned int euid = geteuid();
 
-	opnd1 = *t_wp;
-	(void) t_lex(*++t_wp);
-	op = t_wp_op;
+	if (stat(path, &st) < 0)
+		return (-1);
 
-	if ((opnd2 = *++t_wp) == (char *) 0)
-		bb_error_msg_and_die(bb_msg_requires_arg, op->op_text);
+	if (euid == 0) {
+		/* Root can read or write any file. */
+		if (mode != X_OK)
+			return (0);
 
-	switch (op->op_num) {
-	case STREQ:
-		return strcmp(opnd1, opnd2) == 0;
-	case STRNE:
-		return strcmp(opnd1, opnd2) != 0;
-	case STRLT:
-		return strcmp(opnd1, opnd2) < 0;
-	case STRGT:
-		return strcmp(opnd1, opnd2) > 0;
-	case INTEQ:
-		return getn(opnd1) == getn(opnd2);
-	case INTNE:
-		return getn(opnd1) != getn(opnd2);
-	case INTGE:
-		return getn(opnd1) >= getn(opnd2);
-	case INTGT:
-		return getn(opnd1) > getn(opnd2);
-	case INTLE:
-		return getn(opnd1) <= getn(opnd2);
-	case INTLT:
-		return getn(opnd1) < getn(opnd2);
-	case FILNT:
-		return newerf(opnd1, opnd2);
-	case FILOT:
-		return olderf(opnd1, opnd2);
-	case FILEQ:
-		return equalf(opnd1, opnd2);
+		/* Root can execute any file that has any one of the execute
+		   bits set. */
+		if (st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))
+			return (0);
 	}
-	/* NOTREACHED */
-	return 1;
+
+	if (st.st_uid == euid)	/* owner */
+		mode <<= 6;
+	else if (is_a_group_member(st.st_gid))
+		mode <<= 3;
+
+	if (st.st_mode & mode)
+		return (0);
+
+	return (-1);
 }
 
 static int filstat(char *nm, enum token mode)
@@ -423,52 +372,7 @@
 	return ((s.st_mode & i) != 0);
 }
 
-static enum token t_lex(char *s)
-{
-	struct t_op const *op = ops;
 
-	if (s == 0) {
-		t_wp_op = (struct t_op *) 0;
-		return EOI;
-	}
-	while (op->op_text) {
-		if (strcmp(s, op->op_text) == 0) {
-			t_wp_op = op;
-			return op->op_num;
-		}
-		op++;
-	}
-	t_wp_op = (struct t_op *) 0;
-	return OPERAND;
-}
-
-/* atoi with error detection */
-static arith_t getn(const char *s)
-{
-	char *p;
-#ifdef CONFIG_FEATURE_TEST_64
-	long long r;
-#else
-	long r;
-#endif
-
-	errno = 0;
-#ifdef CONFIG_FEATURE_TEST_64
-	r = strtoll(s, &p, 10);
-#else
-	r = strtol(s, &p, 10);
-#endif
-
-	if (errno != 0)
-		bb_error_msg_and_die("%s: out of range", s);
-
-	/*   p = bb_skip_whitespace(p); avoid const warning */
-	if (*(bb_skip_whitespace(p)))
-		bb_error_msg_and_die("%s: bad number", s);
-
-	return r;
-}
-
 static int newerf(const char *f1, const char *f2)
 {
 	struct stat b1, b2;
@@ -494,62 +398,129 @@
 			b1.st_dev == b2.st_dev && b1.st_ino == b2.st_ino);
 }
 
-/* Do the same thing access(2) does, but use the effective uid and gid,
-   and don't make the mistake of telling root that any file is
-   executable. */
-static int test_eaccess(char *path, int mode)
+static int binop(void)
 {
-	struct stat st;
-	unsigned int euid = geteuid();
+	const char *opnd1, *opnd2;
+	struct t_op const *op;
 
-	if (stat(path, &st) < 0)
-		return (-1);
+	opnd1 = *t_wp;
+	(void) t_lex(*++t_wp);
+	op = t_wp_op;
 
-	if (euid == 0) {
-		/* Root can read or write any file. */
-		if (mode != X_OK)
-			return (0);
+	if ((opnd2 = *++t_wp) == (char *) 0)
+		bb_error_msg_and_die(bb_msg_requires_arg, op->op_text);
 
-		/* Root can execute any file that has any one of the execute
-		   bits set. */
-		if (st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))
-			return (0);
+	switch (op->op_num) {
+	case STREQ:
+		return strcmp(opnd1, opnd2) == 0;
+	case STRNE:
+		return strcmp(opnd1, opnd2) != 0;
+	case STRLT:
+		return strcmp(opnd1, opnd2) < 0;
+	case STRGT:
+		return strcmp(opnd1, opnd2) > 0;
+	case INTEQ:
+		return getn(opnd1) == getn(opnd2);
+	case INTNE:
+		return getn(opnd1) != getn(opnd2);
+	case INTGE:
+		return getn(opnd1) >= getn(opnd2);
+	case INTGT:
+		return getn(opnd1) > getn(opnd2);
+	case INTLE:
+		return getn(opnd1) <= getn(opnd2);
+	case INTLT:
+		return getn(opnd1) < getn(opnd2);
+	case FILNT:
+		return newerf(opnd1, opnd2);
+	case FILOT:
+		return olderf(opnd1, opnd2);
+	case FILEQ:
+		return equalf(opnd1, opnd2);
 	}
+	/* NOTREACHED */
+	return 1;
+}
 
-	if (st.st_uid == euid)	/* owner */
-		mode <<= 6;
-	else if (is_a_group_member(st.st_gid))
-		mode <<= 3;
+static arith_t primary(enum token n)
+{
+	arith_t res;
 
-	if (st.st_mode & mode)
-		return (0);
+	if (n == EOI) {
+		bb_error_msg_and_die("argument expected");
+	}
+	if (n == LPAREN) {
+		res = oexpr(t_lex(*++t_wp));
+		if (t_lex(*++t_wp) != RPAREN)
+			bb_error_msg_and_die("closing paren expected");
+		return res;
+	}
+	if (t_wp_op && t_wp_op->op_type == UNOP) {
+		/* unary expression */
+		if (*++t_wp == NULL)
+			bb_error_msg_and_die(bb_msg_requires_arg, t_wp_op->op_text);
+		if (n == STREZ)
+			return strlen(*t_wp) == 0;
+		if (n == STRNZ)
+			return strlen(*t_wp) != 0;
+		if (n == FILTT)
+			return isatty(getn(*t_wp));
+		return filstat(*t_wp, n);
+	}
 
-	return (-1);
-}
+	if (t_lex(t_wp[1]), t_wp_op && t_wp_op->op_type == BINOP) {
+		return binop();
+	}
 
-static void initialize_group_array(void)
-{
-	ngroups = getgroups(0, NULL);
-	group_array = xrealloc(group_array, ngroups * sizeof(gid_t));
-	getgroups(ngroups, group_array);
+	return strlen(*t_wp) > 0;
 }
 
-/* Return non-zero if GID is one that we have in our groups list. */
-static int is_a_group_member(gid_t gid)
+int test_main(int argc, char **argv)
 {
-	register int i;
+	int res;
+	char a10, a11;
 
-	/* Short-circuit if possible, maybe saving a call to getgroups(). */
-	if (gid == getgid() || gid == getegid())
-		return (1);
+	if (strcmp(bb_applet_name, "[[") == 0) {
+		if (strcmp(argv[--argc], "]]"))
+			bb_error_msg_and_die("missing ]]");
+		argv[argc] = NULL;
+	} /* else */
+	if (strcmp(bb_applet_name, "[") == 0) {
+		if (strcmp(argv[--argc], "]"))
+			bb_error_msg_and_die("missing ]");
+		argv[argc] = NULL;
+	}
+	/* Implement special cases from POSIX.2, section 4.62.4 */
+	if (argc == 1)
+		exit(1);
+	if (argc == 2)
+		exit(*argv[1] == '\0');
+	a10 = argv[1][0];
+	a11 = argv[1][1];
+	if (argc == 3)
+		if (a10 == '!' && a11 == '\0') {
+			exit(!(*argv[2] == '\0'));
+		}
+	if (argc == 4)
+		if (a10 != '!' || a11 != '\0') {
+			if (t_lex(argv[2]), t_wp_op && t_wp_op->op_type == BINOP) {
+				t_wp = &argv[1];
+				exit(binop() == 0);
+			}
+		}
+	if (argc == 5)
+		if (a10 == '!' && a11 == '\0') {
+			if (t_lex(argv[3]), t_wp_op && t_wp_op->op_type == BINOP) {
+				t_wp = &argv[2];
+				exit(!(binop() == 0));
+			}
+		}
 
-	if (ngroups == 0)
-		initialize_group_array();
+	t_wp = &argv[1];
+	res = !oexpr(t_lex(*t_wp));
 
-	/* Search through the list looking for GID. */
-	for (i = 0; i < ngroups; i++)
-		if (gid == group_array[i])
-			return (1);
+	if (*t_wp != NULL && *++t_wp != NULL)
+		bb_error_msg_and_die("%s: unknown operand", *t_wp);
 
-	return (0);
+	return (res);
 }
-------------- next part --------------
Index: applets/applets.c
===================================================================
--- applets/applets.c	(revision 15337)
+++ applets/applets.c	(working copy)
@@ -473,17 +473,73 @@
 				  applet_name_compare);
 }
 
+#if ENABLE_FEATURE_APPLET_SPEED
+/* TODO: support more instruction sets and perhaps move this to another file  */
+
+#if defined __x86_64__ || defined __i386__
+typedef unsigned long long cycles_t;
+static cycles_t get_cycles(void)
+{
+	unsigned low, high;
+	unsigned long long ret;
+
+	asm volatile("rdtsc" : "=a" (low), "=d" (high));
+	ret = high;
+	ret = (ret << 32) | low;
+	return ret;
+}
+#elif defined __PPC__
+typedef unsigned long long cycles_t;
+static cycles_t get_cycles(void)
+{
+	cycles_t ret;
+	asm volatile("mftb %0" : "=r" (ret) : );
+	return ret;
+}
+#elif defined __ia64__ || defined __PPC64__
+#include <asm/timex.h>
+#else
+#warning get_cycles not implemented on your architecture.. trying asm/timex.h
+#include <asm/timex.h>
+#endif
+double _cpu_mhz = -1.0;
+static cycles_t _start_time;
+static void _print_used_cycles(void)
+{
+	cycles_t _end_time = get_cycles();
+	const char *unit = (_cpu_mhz > 0.0000001)?"µsec":"ticks";
+	if (0.0000001 > _cpu_mhz)
+		_cpu_mhz = 1;
+	fprintf(stderr, "Took %g %s\n", (_end_time - _start_time) / _cpu_mhz, unit);
+}
+
+#endif /* ENABLE_FEATURE_APPLET_SPEED */
+
 void run_applet_by_name (const char *name, int argc, char **argv)
 {
 	if(ENABLE_FEATURE_SUID_CONFIG) parse_config_file ();
 
 	if(!strncmp(name, "busybox", 7)) busybox_main(argc, argv);
+
+	/* handle applet-timing. Give the current cpu_mhz as --time=12.0 */
+	if (ENABLE_FEATURE_APPLET_SPEED &&
+		argc > 1 && !strncmp(argv[1], "--time", 6)) {
+		extern double _cpu_mhz;
+		if (sscanf(argv[1], "--time=%lf", &_cpu_mhz) != 1)
+			_cpu_mhz = 0.0; /* just report ticks */
+		argc--; argv++;
+	}
+
 	/* Do a binary search to find the applet entry given the name. */
 	applet_using = find_applet_by_name(name);
 	if(applet_using) {
 		bb_applet_name = applet_using->name;
 		if(argc==2 && !strcmp(argv[1], "--help")) bb_show_usage ();
 		if(ENABLE_FEATURE_SUID) check_suid (applet_using);
+		if (ENABLE_FEATURE_APPLET_SPEED && _cpu_mhz > -0.0000001) {
+			atexit(_print_used_cycles);
+			_start_time = get_cycles();
+		}
 		exit ((*(applet_using->main)) (argc, argv));
 	}
 }
Index: applets/busybox.c
===================================================================
--- applets/busybox.c	(revision 15337)
+++ applets/busybox.c	(working copy)
@@ -113,7 +113,6 @@
 	}
 
 	/* Deal with --help.  (Also print help when called with no arguments) */
-
 	if (argc==1 || !strcmp(argv[1],"--help") ) {
 		if (argc>2) {
 			run_applet_by_name(bb_applet_name=argv[2], 2, argv);
Index: Config.in
===================================================================
--- Config.in	(revision 15337)
+++ Config.in	(working copy)
@@ -367,7 +367,6 @@
 	  and run slower, so you should leave this option disabled unless
 	  you are hunting a hard to find memory problem.
 
-
 config CONFIG_NO_DEBUG_LIB
 	bool "None"
 
@@ -379,6 +378,12 @@
 
 endchoice
 
+config CONFIG_FEATURE_APPLET_SPEED
+	bool "Time applet execution speed"
+	default n
+	help
+	  Say Y here if you wish to measure the execution time of applets.
+
 config CONFIG_DEBUG_YANK_SUSv2
 	bool "Disable obsolete features removed before SUSv3?"
 	default y


More information about the busybox mailing list