[pacman-dev] [PATCH 1/2] Improve mbasename performance
Rather than roll our own, use strrchr() instead, which glibc may have a better implementation than the simple iteration method we were using. Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/query.c | 4 ++-- src/pacman/util.c | 18 +++++------------- src/pacman/util.h | 2 +- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/pacman/query.c b/src/pacman/query.c index dea309a..f599360 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -109,8 +109,8 @@ static int query_fileowner(alpm_list_t *targets) for(t = targets; t; t = alpm_list_next(t)) { int found = 0; filename = strdup(alpm_list_getdata(t)); - char *bname, *dname, *rpath; - const char *root; + char *dname, *rpath; + const char *root, *bname; struct stat buf; alpm_list_t *i, *j; diff --git a/src/pacman/util.c b/src/pacman/util.c index d91d1d4..0377bf7 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -157,25 +157,17 @@ int rmrf(const char *path) } /** Parse the basename of a program from a path. -* Grabbed from the uClibc source. * @param path path to parse basename from * * @return everything following the final '/' */ -char *mbasename(const char *path) +const char *mbasename(const char *path) { - const char *s; - const char *p; - - p = s = path; - - while (*s) { - if (*s++ == '/') { - p = s; - } + const char *last = strrchr(path, '/'); + if(last) { + return(last + 1); } - - return (char *)p; + return(path); } /** Parse the dirname of a program from a path. diff --git a/src/pacman/util.h b/src/pacman/util.h index a5c382d..78fe5b5 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -44,7 +44,7 @@ int trans_release(void); int needs_root(void); int getcols(void); int rmrf(const char *path); -char *mbasename(const char *path); +const char *mbasename(const char *path); char *mdirname(const char *path); void indentprint(const char *str, int indent); char *strtoupper(char *str); -- 1.7.3.5
Clean up some of the code by doing less string copying and printing. This is accomplished by either doing it after we know we need it, or taking advantage of the fact that some strings never change such as the root directory prefix. Also, fix an issue where a file at the root level (e.g. /foobar) could not be queried. End result is a much faster user experience when combined with the mbasename() changes. These timings are for looking up 113 files in /etc/, some of which are owned and some which are not. $ find /etc -maxdepth 1 -type f | xargs time pacman -Qo >/dev/null 6.10user 0.05system 0:06.17elapsed 99%CPU (0avgtext+0avgdata 131040maxresident)k 0inputs+0outputs (0major+9436minor)pagefaults 0swaps $ find /etc -maxdepth 1 -type f | xargs time ./src/pacman/.libs/lt-pacman -Qo >/dev/null 0.86user 0.04system 0:00.92elapsed 99%CPU (0avgtext+0avgdata 131120maxresident)k 0inputs+0outputs (0major+9436minor)pagefaults 0swaps I'll take a 600% increase in speed. Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/query.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/pacman/query.c b/src/pacman/query.c index f599360..fc6a2a5 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -39,11 +39,11 @@ extern pmdb_t *db_local; -static char *resolve_path(const char* file) +static char *resolve_path(const char *file) { char *str = NULL; - str = calloc(PATH_MAX+1, sizeof(char)); + str = calloc(PATH_MAX + 1, sizeof(char)); if(!str) { return(NULL); } @@ -97,7 +97,10 @@ static int search_path(char **filename, struct stat *bufptr) static int query_fileowner(alpm_list_t *targets) { int ret = 0; - char *filename; + char path[PATH_MAX]; + const char *root; + char *append; + size_t max_length; alpm_list_t *t; /* This code is here for safety only */ @@ -106,13 +109,22 @@ static int query_fileowner(alpm_list_t *targets) return(1); } + /* Set up our root path buffer. We only need to copy the location of root in + * once, then we can just overwrite whatever file was there on the previous + * iteration. */ + root = alpm_option_get_root(); + strncpy(path, root, PATH_MAX - 1); + append = path + strlen(path); + max_length = PATH_MAX - (append - path) - 1; + for(t = targets; t; t = alpm_list_next(t)) { + char *filename, *dname, *rpath; + const char *bname; + struct stat buf; + alpm_list_t *i; int found = 0; + filename = strdup(alpm_list_getdata(t)); - char *dname, *rpath; - const char *root, *bname; - struct stat buf; - alpm_list_t *i, *j; if(lstat(filename, &buf) == -1) { /* if it is not a path but a program name, then check in PATH */ @@ -144,32 +156,38 @@ static int query_fileowner(alpm_list_t *targets) bname = mbasename(filename); dname = mdirname(filename); rpath = resolve_path(dname); - free(dname); - if(!rpath) { + /* this odd conditional is to ensure files in '/' can be checked */ + if(!rpath && strcmp(dname, "") != 0) { pm_fprintf(stderr, PM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"), filename, strerror(errno)); free(filename); + free(dname); free(rpath); ret++; continue; } - - root = alpm_option_get_root(); + free(dname); for(i = alpm_db_get_pkgcache(db_local); i && !found; i = alpm_list_next(i)) { + alpm_list_t *j; pmpkg_t *info = alpm_list_getdata(i); for(j = alpm_pkg_get_files(info); j && !found; j = alpm_list_next(j)) { - char path[PATH_MAX], *ppath, *pdname; - snprintf(path, PATH_MAX, "%s%s", - root, (const char *)alpm_list_getdata(j)); + char *ppath, *pdname; + const char *pkgfile = alpm_list_getdata(j); /* avoid the costly resolve_path usage if the basenames don't match */ - if(strcmp(mbasename(path), bname) != 0) { + if(strcmp(mbasename(pkgfile), bname) != 0) { continue; } + if(strlen(pkgfile) > max_length) { + pm_fprintf(stderr, PM_LOG_ERROR, _("Path too long: %s%s\n"), root, pkgfile); + } + /* concatenate our file and the root path */ + strcpy(append, pkgfile); + pdname = mdirname(path); ppath = resolve_path(pdname); free(pdname); -- 1.7.3.5
participants (1)
-
Dan McGee