[pacman-dev] [PATCH 2/2] Do not replicate files list when removing packages

[Dan McGee]

On Fri, Jun 24, 2011 at 6:22 PM, Allan McRae <allan at archlinux.org> wrote:
> This saves replicating the list of files in a package that is being
> removed which can be quite large.
>
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---
>
> The tradeoff here is a slightly less readable loop, but duplicating that list
> just struck me as wasteful.

I agree with the idea; I agree the implementation is a tad crazy.

What if we added an alpm_list_prev() function instead? This wouldn't
directly mirror alpm_list_next() due to the whole "quick last item"
bit, so instead it would be something like:
    alpm_list_t *alpm_list_prev(alpm_list_t *list, alpm_list_t *item);
and the ickyness would be wrapped up in here, special casing if list == item.

Then this patch would be a one-liner change to the loop to something
like the following:
for(lp = alpm_list_last(files); lp; lp = alpm_list_prev(files, lp)) {

>  lib/libalpm/remove.c |   41 +++++++++++++++++++++++------------------
>  1 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
> index cc6daea..2d75a84 100644
> --- a/lib/libalpm/remove.c
> +++ b/lib/libalpm/remove.c
> @@ -285,8 +285,7 @@ static void unlink_file(pmhandle_t *handle, pmpkg_t *info, const char *filename,
>  int _alpm_upgraderemove_package(pmhandle_t *handle,
>                pmpkg_t *oldpkg, pmpkg_t *newpkg)
>  {
> -       alpm_list_t *skip_remove, *b;
> -       alpm_list_t *newfiles, *lp;
> +       alpm_list_t *skip_remove, *b, *lp;
>        size_t filenum=0;
>        alpm_list_t *files = alpm_pkg_get_files(oldpkg);
>        const char *pkgname = alpm_pkg_get_name(oldpkg);
> @@ -329,11 +328,15 @@ int _alpm_upgraderemove_package(pmhandle_t *handle,
>        _alpm_log(handle, PM_LOG_DEBUG, "removing %ld files\n", (unsigned long)filenum);
>
>        /* iterate through the list backwards, unlinking files */
> -       newfiles = alpm_list_reverse(files);
> -       for(lp = newfiles; lp; lp = alpm_list_next(lp)) {
> -               unlink_file(handle, oldpkg, lp->data, skip_remove, 0);
> +       if(files) {
> +               alpm_list_t *last = alpm_list_last(files);
> +               lp = last;
> +               do {
> +                       unlink_file(handle, oldpkg, lp->data, skip_remove, 0);
> +                       lp = lp->prev;
> +               } while (lp != last);
>        }
> -       alpm_list_free(newfiles);
> +
>        FREELIST(skip_remove);
>
>  db:
> @@ -390,7 +393,6 @@ int _alpm_remove_packages(pmhandle_t *handle)
>
>                if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) {
>                        alpm_list_t *files = alpm_pkg_get_files(info);
> -                       alpm_list_t *newfiles;
>                        size_t filenum=0;
>
>                        for(lp = files; lp; lp = lp->next) {
> @@ -409,18 +411,21 @@ int _alpm_remove_packages(pmhandle_t *handle)
>                                        pkg_count, (pkg_count - targcount + 1));
>
>                        /* iterate through the list backwards, unlinking files */
> -                       newfiles = alpm_list_reverse(files);
> -                       for(lp = newfiles; lp; lp = alpm_list_next(lp)) {
> -                               int percent;
> -                               unlink_file(handle, info, lp->data, NULL, trans->flags & PM_TRANS_FLAG_NOSAVE);
> -
> -                               /* update progress bar after each file */
> -                               percent = (position * 100) / filenum;
> -                               PROGRESS(trans, PM_TRANS_PROGRESS_REMOVE_START, info->name,
> -                                               percent, pkg_count, (pkg_count - targcount + 1));
> -                               position++;
> +                       if(files) {
> +                               alpm_list_t *last = alpm_list_last(files);
> +                               lp = last;
> +                               do {
> +                                       int percent;
> +                                       unlink_file(handle, info, lp->data, NULL, trans->flags & PM_TRANS_FLAG_NOSAVE);
> +
> +                                       /* update progress bar after each file */
> +                                       percent = (position * 100) / filenum;
> +                                       PROGRESS(trans, PM_TRANS_PROGRESS_REMOVE_START, info->name,
> +                                                       percent, pkg_count, (pkg_count - targcount + 1));
> +                                       position++;
> +                                       lp = lp->prev;
> +                               } while (lp != last);
>                        }
> -                       alpm_list_free(newfiles);
>                }
>
>                /* set progress to 100% after we finish unlinking files */
> --
> 1.7.5.4
>
>
>