On 06/14/2011 06:15 PM, Ed W wrote:
> On 14/06/2011 16:10, Timo Teräs wrote:
>> The proper thing to do still is the implement the binary file format.
>> This is just trying to do micro-optimizations compared to it.
>
> Could be - however, Lauri's results were: 28% faster using binary
> modutils and 23% faster using text modutils. Given that our "readline"
> function is used in quite a few places in busybox, getting this nailed
> has the potential to cause significant speedups in other places such as
> sed (often used on reasonably sized files?)
>
> I hope you will consider looking slightly further into this? It does
> seem like low hanging performance fruit that would have an effect more
> widely than just "modprobe"?
Looking at callgrind, it says that the line reader is the biggest CPU
hog. I'm not really sure how to further speed it up other than using the
unlocked getc. Also, glibc vs. uclibc implementations have difference.
The second thing that popped up in callgrind is the way bb modprobe does
the modentry caching. Apparently the amount of modentries can blowup
depending on how you've configured. Actually, this can even be the
slowest thing depending on the exact thing how things go.
Please try the inlined patch below.
diff --git a/modutils/modprobe.c b/modutils/modprobe.c
index 678f4be..b3767ac 100644
--- a/modutils/modprobe.c
+++ b/modutils/modprobe.c
@@ -157,8 +157,10 @@ struct module_entry { /* I'll call it ME. */
llist_t *deps; /* strings. modules we depend on */
};
+#define DB_HASH_SIZE 256
+
struct globals {
- llist_t *db; /* MEs of all modules ever seen (caching for speed) */
+ llist_t *db[DB_HASH_SIZE]; /* MEs of all modules ever seen (caching
for speed) */
llist_t *probes; /* MEs of module(s) requested on cmdline */
char *cmdline_mopts; /* module options from cmdline */
int num_unresolved_deps;
@@ -195,9 +197,14 @@ static struct module_entry *helper_get_module(const
char *module, int create)
char modname[MODULE_NAME_LEN];
struct module_entry *e;
llist_t *l;
+ unsigned int hash = 5381, i;
filename2modname(module, modname);
- for (l = G.db; l != NULL; l = l->link) {
+ for (i = 0; modname[i]; i++)
+ hash = ((hash << 5) + hash) + modname[i];
+ hash %= DB_HASH_SIZE;
+
+ for (l = G.db[hash]; l != NULL; l = l->link) {
e = (struct module_entry *) l->data;
if (strcmp(e->modname, modname) == 0)
return e;
@@ -207,7 +214,7 @@ static struct module_entry *helper_get_module(const
char *module, int create)
e = xzalloc(sizeof(*e));
e->modname = xstrdup(modname);
- llist_add_to(&G.db, e);
+ llist_add_to(&G.db[hash], e);
return e;
}
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
