[pacman-dev] pacman -R speed-up (empty cache)
Hi! Around this -S reworking, I looked into "-R foo" code, and I noticed that we search for foo group, THEN literal... This forces pacman to read all desc files. Wow. pacman -R group is not a common task... You may say, that I'm cheating, because we will need to check dependencies which loads the localdb. But this task requires depends files only. I temporarily disabled this group removal stuff (pacman.static.new) Let's test (in both case zip is installed): # echo 3 > /proc/sys/vm/drop_caches && time pacman.static.new -R zip real 0m7.697s user 0m0.056s sys 0m0.091s # echo 3 > /proc/sys/vm/drop_caches && time pacman.static.old -R zip real 0m11.822s user 0m0.075s sys 0m0.126s Check what would happen, if -R wouldn't deal with groups at all (my preferred -Rg group way; xyz is not installed): # echo 3 > /proc/sys/vm/drop_caches && time pacman.static.new -R xyz real 0m1.130s user 0m0.009s sys 0m0.011s # echo 3 > /proc/sys/vm/drop_caches && time pacman.static.old -R xyz real 0m7.275s user 0m0.051s sys 0m0.081s
From efb3f3e401a2d57480ff205bfb0ede11a920091b Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Sat, 5 Jul 2008 22:30:14 +0200 Subject: [PATCH] remove_addtarget rework (in front-end) Now "pacman -R foo" first searches for literal, and then for group. This is faster in most cases, see: http://www.archlinux.org/pipermail/pacman-dev/2008-July/012311.html "-R group" implementation was broken, since alpm_grp_get_pkgs returns with an pmpkg_t list, not a string list. Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- src/pacman/remove.c | 94 +++++++++++++++++++++++++------------------------- 1 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 4fe9bc8..784c998 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -42,58 +42,62 @@ extern pmdb_t *db_local; */ int pacman_remove(alpm_list_t *targets) { - alpm_list_t *i, *data = NULL, *finaltargs = NULL; int retval = 0; + alpm_list_t *i, *data = NULL; if(targets == NULL) { pm_printf(PM_LOG_ERROR, _("no targets specified (use -h for help)\n")); return(1); } - /* If the target is a group, ask if its packages should be removed - * (the library can't remove groups for now) - */ - for(i = targets; i; i = alpm_list_next(i)) { - pmgrp_t *grp = alpm_db_readgrp(db_local, alpm_list_getdata(i)); - if(grp) { - int all; - const alpm_list_t *p, *pkgnames = alpm_grp_get_pkgs(grp); - - printf(_(":: group %s:\n"), alpm_grp_get_name(grp)); - list_display(" ", pkgnames); - all = yesno(1, _(" Remove whole content?")); - - for(p = pkgnames; p; p = alpm_list_next(p)) { - char *pkg = alpm_list_getdata(p); - if(all || yesno(1, _(":: Remove %s from group %s?"), pkg, (char *)alpm_list_getdata(i))) { - finaltargs = alpm_list_add(finaltargs, strdup(pkg)); - } - } - } else { - /* not a group, so add it to the final targets */ - finaltargs = alpm_list_add(finaltargs, strdup(alpm_list_getdata(i))); - } - } - - /* Step 1: create a new transaction */ + /* Step 0: create a new transaction */ if(trans_init(PM_TRANS_TYPE_REMOVE, config->flags) == -1) { - FREELIST(finaltargs); return(1); } - /* add targets to the created transaction */ - printf(_("loading package data...\n")); - for(i = finaltargs; i; i = alpm_list_next(i)) { + /* Step 1: add targets to the created transaction */ + for(i = targets; i; i = alpm_list_next(i)) { char *targ = alpm_list_getdata(i); if(alpm_trans_addtarget(targ) == -1) { - pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", - targ, alpm_strerrorlast()); - trans_release(); - FREELIST(finaltargs); - return(1); + if(pm_errno == PM_ERR_PKG_NOT_FOUND) { + printf(_("%s not found, searching for group...\n"), targ); + pmgrp_t *grp = alpm_db_readgrp(db_local, targ); + if(grp == NULL) { + pm_fprintf(stderr, PM_LOG_ERROR, _("'%s': not found in local db\n"), targ); + retval = 1; + goto cleanup; + } else { + alpm_list_t *p, *pkgnames = NULL; + /* convert packages to package names */ + for(p = alpm_grp_get_pkgs(grp); p; p = alpm_list_next(p)) { + pmpkg_t *pkg = alpm_list_getdata(p); + pkgnames = alpm_list_add(pkgnames, (void *)alpm_pkg_get_name(pkg)); + } + printf(_(":: group %s:\n"), targ); + list_display(" ", pkgnames); + int all = yesno(1, _(" Remove whole content?")); + for(p = pkgnames; p; p = alpm_list_next(p)) { + char *pkgn = alpm_list_getdata(p); + if(all || yesno(1, _(":: Remove %s from group %s?"), pkgn, targ)) { + if(alpm_trans_addtarget(pkgn) == -1) { + pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", targ, + alpm_strerrorlast()); + retval = 1; + alpm_list_free(pkgnames); + goto cleanup; + } + } + } + alpm_list_free(pkgnames); + } + } else { + pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", targ, alpm_strerrorlast()); + retval = 1; + goto cleanup; + } } } - + /* Step 2: prepare the transaction based on its type, targets and flags */ if(alpm_trans_prepare(&data) == -1) { pm_fprintf(stderr, PM_LOG_ERROR, _("failed to prepare transaction (%s)\n"), @@ -113,9 +117,8 @@ int pacman_remove(alpm_list_t *targets) default: break; } - trans_release(); - FREELIST(finaltargs); - return(1); + retval = 1; + goto cleanup; } /* Warn user in case of dangerous operation */ @@ -133,9 +136,8 @@ int pacman_remove(alpm_list_t *targets) FREELIST(lst); /* get confirmation */ if(yesno(1, _("\nDo you want to remove these packages?")) == 0) { - trans_release(); - FREELIST(finaltargs); - return(1); + retval = 1; + goto cleanup; } printf("\n"); } @@ -144,16 +146,14 @@ int pacman_remove(alpm_list_t *targets) if(alpm_trans_commit(NULL) == -1) { pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)\n"), alpm_strerrorlast()); - trans_release(); - FREELIST(finaltargs); - return(1); + retval = 1; } /* Step 4: release transaction resources */ +cleanup: if(trans_release() == -1) { retval = 1; } - FREELIST(finaltargs); return(retval); } -- 1.5.6.1
On Sat, Jul 5, 2008 at 3:49 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
From efb3f3e401a2d57480ff205bfb0ede11a920091b Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Sat, 5 Jul 2008 22:30:14 +0200 Subject: [PATCH] remove_addtarget rework (in front-end)
Now "pacman -R foo" first searches for literal, and then for group. This is faster in most cases, see: http://www.archlinux.org/pipermail/pacman-dev/2008-July/012311.html
"-R group" implementation was broken, since alpm_grp_get_pkgs returns with an pmpkg_t list, not a string list. Perhaps we need a group removal pactest?
Please wrap at 76 chars as Xavier alluded to in the other thread regarding commit messages- I fixed the rest of yours but since this will need resubmitting you should be able to fix it here. Wrapping at 76 makes git-log in the terminal a lot cleaner and easier to follow.
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- src/pacman/remove.c | 94 +++++++++++++++++++++++++------------------------- 1 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 4fe9bc8..784c998 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -42,58 +42,62 @@ extern pmdb_t *db_local; */ int pacman_remove(alpm_list_t *targets) { - alpm_list_t *i, *data = NULL, *finaltargs = NULL; int retval = 0; + alpm_list_t *i, *data = NULL;
if(targets == NULL) { pm_printf(PM_LOG_ERROR, _("no targets specified (use -h for help)\n")); return(1); }
- /* If the target is a group, ask if its packages should be removed - * (the library can't remove groups for now) - */ - for(i = targets; i; i = alpm_list_next(i)) { - pmgrp_t *grp = alpm_db_readgrp(db_local, alpm_list_getdata(i)); - if(grp) { - int all; - const alpm_list_t *p, *pkgnames = alpm_grp_get_pkgs(grp); - - printf(_(":: group %s:\n"), alpm_grp_get_name(grp)); - list_display(" ", pkgnames); - all = yesno(1, _(" Remove whole content?")); - - for(p = pkgnames; p; p = alpm_list_next(p)) { - char *pkg = alpm_list_getdata(p); - if(all || yesno(1, _(":: Remove %s from group %s?"), pkg, (char *)alpm_list_getdata(i))) { - finaltargs = alpm_list_add(finaltargs, strdup(pkg)); - } - } - } else { - /* not a group, so add it to the final targets */ - finaltargs = alpm_list_add(finaltargs, strdup(alpm_list_getdata(i))); - } - } - - /* Step 1: create a new transaction */ + /* Step 0: create a new transaction */ if(trans_init(PM_TRANS_TYPE_REMOVE, config->flags) == -1) { - FREELIST(finaltargs); return(1); }
- /* add targets to the created transaction */ - printf(_("loading package data...\n")); - for(i = finaltargs; i; i = alpm_list_next(i)) { + /* Step 1: add targets to the created transaction */ + for(i = targets; i; i = alpm_list_next(i)) { char *targ = alpm_list_getdata(i); if(alpm_trans_addtarget(targ) == -1) { - pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", - targ, alpm_strerrorlast()); - trans_release(); - FREELIST(finaltargs); - return(1); + if(pm_errno == PM_ERR_PKG_NOT_FOUND) { + printf(_("%s not found, searching for group...\n"), targ);
Please use pm_printf with an applicable level- INFO perhaps?
+ pmgrp_t *grp = alpm_db_readgrp(db_local, targ); + if(grp == NULL) { + pm_fprintf(stderr, PM_LOG_ERROR, _("'%s': not found in local db\n"), targ); + retval = 1; + goto cleanup; + } else { + alpm_list_t *p, *pkgnames = NULL; + /* convert packages to package names */ + for(p = alpm_grp_get_pkgs(grp); p; p = alpm_list_next(p)) { + pmpkg_t *pkg = alpm_list_getdata(p); + pkgnames = alpm_list_add(pkgnames, (void *)alpm_pkg_get_name(pkg)); + } + printf(_(":: group %s:\n"), targ); + list_display(" ", pkgnames); + int all = yesno(1, _(" Remove whole content?")); + for(p = pkgnames; p; p = alpm_list_next(p)) { + char *pkgn = alpm_list_getdata(p); + if(all || yesno(1, _(":: Remove %s from group %s?"), pkgn, targ)) { + if(alpm_trans_addtarget(pkgn) == -1) { + pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", targ, + alpm_strerrorlast()); + retval = 1; + alpm_list_free(pkgnames); + goto cleanup; + } + } + } + alpm_list_free(pkgnames); + } + } else { + pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", targ, alpm_strerrorlast()); + retval = 1; + goto cleanup; + } } } - + /* Step 2: prepare the transaction based on its type, targets and flags */ if(alpm_trans_prepare(&data) == -1) { pm_fprintf(stderr, PM_LOG_ERROR, _("failed to prepare transaction (%s)\n"), @@ -113,9 +117,8 @@ int pacman_remove(alpm_list_t *targets) default: break; } - trans_release(); - FREELIST(finaltargs); - return(1); + retval = 1; + goto cleanup; }
/* Warn user in case of dangerous operation */ @@ -133,9 +136,8 @@ int pacman_remove(alpm_list_t *targets) FREELIST(lst); /* get confirmation */ if(yesno(1, _("\nDo you want to remove these packages?")) == 0) { - trans_release(); - FREELIST(finaltargs); - return(1); + retval = 1; + goto cleanup; } printf("\n"); } @@ -144,16 +146,14 @@ int pacman_remove(alpm_list_t *targets) if(alpm_trans_commit(NULL) == -1) { pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)\n"), alpm_strerrorlast()); - trans_release(); - FREELIST(finaltargs); - return(1); + retval = 1; }
/* Step 4: release transaction resources */ +cleanup: if(trans_release() == -1) { retval = 1; } - FREELIST(finaltargs); return(retval); }
-- 1.5.6.1
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev
Please use pm_printf with an applicable level- INFO perhaps? We have no INFO level, which is a bit funny, indeed (we simply use WARNING from back-end in these cases). So I don't know what to do here. When I wrote this patch, I just looked around in sync.c and I found a lot of pure printf there. So we may want to introduce PM_LOG_INFO in separate commit, and replace printfs to pm_printf in front-end. Is this OK or did I misinterpret something?
I just rewrote the 2 remaining commit message for now, and added -R group pactest. Bye
Hi! OK. We saw, that we should load as few desc files as possible. We can gain one more benefit if we reduce desc file read (over speed-up): we spare some memory (we have "on-demand" pkgcache, once we read desc, it will stay in memory...). Btw, desc files are for packager, build date, etc. not to load them for usual operations (they are quite long, anyway) imho. We get many complaints about pacman speed... (Khm. These complaints are valid.) I did some tests, and I found that pacman -S and pacman -R is not so bad (~10 sec on my machine with empty cache; I'm testing -S with -Sp always). But -Su is quite "terrible", simply 0.5-1 min (-Sup!). I did some debugging, I found that all desc files are read in sync. Why? alpm_sysupgrade: 1. %REPLACES% 2. %FORCE% (this is reason for the "installed" syncpkgs only) If you are unlucky, pacman reads many depends files in sync. Why? resolvedeps: The search for literal rule is crucial here. get_frompkgcache is fast, because it doesn't require desc, depends or files. And in most cases the found literal is a satisfier. You are unlucky, if resolvedeps must search for provision (freeglut provides glut), then pacman will read depends files until it finds the provision. (So quick tip for packagers: Use literal package in %DEPENDS%, if possible ;-) I hacked my pacman, I disabled 1. and 2. Result: _3 times_ faster -Sup. I just brought this up, maybe somebody have an idea about faster implementation of 1. and 2. (it's easier to modify syncdbs than localdb) and maybe this why pacman reads db files discussion was interesting. Bye PS: Status of FS#8586. Speed tests there?
participants (2)
-
Dan McGee
-
Nagy Gabor