From b6a576223f465dfec0b1fade772099fecff68300 Mon Sep 17 00:00:00 2001 From: Duncan Overbruck Date: Sat, 12 Dec 2020 23:03:06 +0100 Subject: [PATCH 1/3] lib: replace xbps_file_hash_check_dictionary with transaction_file lookup xbps_file_hash_check_dictionary was called for every file that is getting unpacked, because the files list is an array it iterated over the whole files array to find the matching file. With a lot of files this is really slow and a lot of time was spend in locking the proplib array and iterating over it. Time from my Ryzen 3700X system with nvme disk updating texlive-fontsextra with files 86375: time xbps-install -y texlive-fontsextra 6m34.61s real 6m27.71s user 0m03.95s system And with this patch: time xbps-install -y texlive-fontsextra 0m08.40s real 0m07.34s user 0m00.98s system --- include/xbps_api_impl.h | 26 ++++++++++++++-- lib/package_unpack.c | 18 ++++++----- lib/transaction_files.c | 50 +++++++++++++---------------- lib/util_hash.c | 69 ----------------------------------------- 4 files changed, 56 insertions(+), 107 deletions(-) diff --git a/include/xbps_api_impl.h b/include/xbps_api_impl.h index 8d3a9aa9e..fe10b0227 100644 --- a/include/xbps_api_impl.h +++ b/include/xbps_api_impl.h @@ -95,6 +95,26 @@ #define __arraycount(x) (sizeof(x) / sizeof(*x)) #endif +typedef enum transaction_file_type { + TYPE_LINK = 1, + TYPE_DIR, + TYPE_FILE, + TYPE_CONFFILE, +} transaction_file_type_t; + +struct transaction_file { + const char *pkgname; + const char *pkgver; + char *sha256; + const char *target; + uint64_t size; + transaction_file_type_t type; + unsigned int index; + bool preserve; + bool update; + bool removepkg; +}; + /** * @private */ @@ -135,14 +155,16 @@ bool HIDDEN xbps_transaction_store(struct xbps_handle *, xbps_array_t, xbps_dict int HIDDEN xbps_transaction_init(struct xbps_handle *); int HIDDEN xbps_transaction_files(struct xbps_handle *, xbps_object_iterator_t); +void xbps_transaction_files_free(void); int HIDDEN xbps_transaction_fetch(struct xbps_handle *, xbps_object_iterator_t); int HIDDEN xbps_transaction_pkg_deps(struct xbps_handle *, xbps_array_t, xbps_dictionary_t); +struct transaction_file HIDDEN * +xbps_transaction_file_new(struct xbps_handle *xhp UNUSED, const char *path); + char HIDDEN *xbps_get_remote_repo_string(const char *); int HIDDEN xbps_repo_sync(struct xbps_handle *, const char *); -int HIDDEN xbps_file_hash_check_dictionary(struct xbps_handle *, - xbps_dictionary_t, const char *, const char *); int HIDDEN xbps_file_exec(struct xbps_handle *, const char *, ...); void HIDDEN xbps_set_cb_fetch(struct xbps_handle *, off_t, off_t, off_t, const char *, bool, bool, bool); diff --git a/lib/package_unpack.c b/lib/package_unpack.c index e947a4ebf..6daadb5ee 100644 --- a/lib/package_unpack.c +++ b/lib/package_unpack.c @@ -266,6 +266,7 @@ unpack_archive(struct xbps_handle *xhp, * Unpack all files on archive now. */ for (;;) { + struct transaction_file *file; ar_rv = archive_read_next_header(ar, &entry); if (ar_rv == ARCHIVE_EOF || ar_rv == ARCHIVE_FATAL) break; @@ -342,15 +343,17 @@ unpack_archive(struct xbps_handle *xhp, continue; } + assert(*entry_pname == '.'); + file = xbps_transaction_file_new(xhp, entry_pname+1); + assert(file); + /* * Check if current entry is a configuration file, * that should be kept. */ - if (!force && (entry_type == AE_IFREG)) { - buf = strchr(entry_pname, '.') + 1; - assert(buf != NULL); - keep_conf_file = xbps_entry_is_a_conf_file(binpkg_filesd, buf); - } + keep_conf_file = !force + && (entry_type == AE_IFREG) + && file->type == TYPE_CONFFILE; /* * If file to be extracted does not match the file type of @@ -387,8 +390,9 @@ unpack_archive(struct xbps_handle *xhp, } rv = 0; } else { - rv = xbps_file_hash_check_dictionary( - xhp, binpkg_filesd, "files", buf); + xbps_dbg_printf(xhp, "%s: %s\n", entry_pname, file->sha256); + assert(file->sha256); + rv = xbps_file_sha256_check(entry_pname, file->sha256); if (rv == -1) { /* error */ xbps_dbg_printf(xhp, diff --git a/lib/transaction_files.c b/lib/transaction_files.c index 9dd4a7699..d45f4e3d7 100644 --- a/lib/transaction_files.c +++ b/lib/transaction_files.c @@ -33,28 +33,10 @@ #include "xbps_api_impl.h" #include "uthash.h" -enum type { - TYPE_LINK = 1, - TYPE_DIR, - TYPE_FILE, - TYPE_CONFFILE, -}; - struct item { char *file; size_t len; - struct { - const char *pkgname; - const char *pkgver; - char *sha256; - const char *target; - uint64_t size; - enum type type; - unsigned int index; - bool preserve; - bool update; - bool removepkg; - } old, new; + struct transaction_file old, new; bool deleted; UT_hash_handle hh; }; @@ -114,7 +96,7 @@ addItem(const char *file) } static const char * -typestr(enum type typ) +typestr(transaction_file_type_t typ) { switch (typ) { case TYPE_LINK: return "symlink"; @@ -420,8 +402,8 @@ collect_obsoletes(struct xbps_handle *xhp) static int collect_file(struct xbps_handle *xhp, const char *file, size_t size, const char *pkgname, const char *pkgver, unsigned int idx, - const char *sha256, enum type type, bool update, bool removepkg, - bool preserve, bool removefile, const char *target) + const char *sha256, transaction_file_type_t type, bool update, + bool removepkg, bool preserve, bool removefile, const char *target) { struct item *item; @@ -526,6 +508,8 @@ collect_file(struct xbps_handle *xhp, const char *file, size_t size, item->new.update = update; item->new.removepkg = removepkg; item->new.target = target; + if (sha256) + item->new.sha256 = strdup(sha256); } if (item->old.type && item->new.type) { /* @@ -565,8 +549,7 @@ collect_files(struct xbps_handle *xhp, xbps_dictionary_t d, for (i = 0; i < xbps_array_count(a); i++) { filed = xbps_array_get(a, i); xbps_dictionary_get_cstring_nocopy(filed, "file", &file); - if (removefile) - xbps_dictionary_get_cstring_nocopy(filed, "sha256", &sha256); + xbps_dictionary_get_cstring_nocopy(filed, "sha256", &sha256); size = 0; xbps_dictionary_get_uint64(filed, "size", &size); rv = collect_file(xhp, file, size, pkgname, pkgver, idx, sha256, @@ -585,8 +568,7 @@ collect_files(struct xbps_handle *xhp, xbps_dictionary_t d, xbps_dictionary_get_cstring_nocopy(filed, "file", &file); size = 0; xbps_dictionary_get_uint64(filed, "size", &size); - if (removefile) - xbps_dictionary_get_cstring_nocopy(filed, "sha256", &sha256); + xbps_dictionary_get_cstring_nocopy(filed, "sha256", &sha256); #if 0 /* XXX: how to handle conf_file size */ if (removefile && stat(file, &st) != -1 && size != (uint64_t)st.st_size) @@ -746,8 +728,8 @@ pathcmp(const void *l1, const void *l2) return (a->len < b->len) - (b->len < a->len); } -static void -cleanup(void) +void +xbps_transaction_files_free(void) { struct item *item, *itmp; @@ -858,6 +840,16 @@ xbps_transaction_files(struct xbps_handle *xhp, xbps_object_iterator_t iter) return rv; rv = collect_obsoletes(xhp); - cleanup(); return rv; } + +struct transaction_file HIDDEN * +xbps_transaction_file_new(struct xbps_handle *xhp UNUSED, const char *path) +{ + struct item *item; + + if ((item = lookupItem(path)) == NULL) + return NULL; + + return &item->new; +} diff --git a/lib/util_hash.c b/lib/util_hash.c index 4b1de715b..392582f81 100644 --- a/lib/util_hash.c +++ b/lib/util_hash.c @@ -209,72 +209,3 @@ xbps_file_sha256_check(const char *file, const char *sha256) return 0; } - -static const char * -file_hash_dictionary(xbps_dictionary_t d, const char *key, const char *file) -{ - xbps_object_t obj; - xbps_object_iterator_t iter; - const char *curfile = NULL, *sha256 = NULL; - - assert(xbps_object_type(d) == XBPS_TYPE_DICTIONARY); - assert(key != NULL); - assert(file != NULL); - - iter = xbps_array_iter_from_dict(d, key); - if (iter == NULL) { - errno = ENOENT; - return NULL; - } - while ((obj = xbps_object_iterator_next(iter)) != NULL) { - xbps_dictionary_get_cstring_nocopy(obj, - "file", &curfile); - if (strcmp(file, curfile) == 0) { - /* file matched */ - xbps_dictionary_get_cstring_nocopy(obj, - "sha256", &sha256); - break; - } - } - xbps_object_iterator_release(iter); - if (sha256 == NULL) - errno = ENOENT; - - return sha256; -} - -int HIDDEN -xbps_file_hash_check_dictionary(struct xbps_handle *xhp, - xbps_dictionary_t d, - const char *key, - const char *file) -{ - const char *sha256d = NULL; - char *buf; - int rv; - - assert(xbps_object_type(d) == XBPS_TYPE_DICTIONARY); - assert(key != NULL); - assert(file != NULL); - - if ((sha256d = file_hash_dictionary(d, key, file)) == NULL) { - if (errno == ENOENT) - return 1; /* no match, file not found */ - - return -1; /* error */ - } - - if (strcmp(xhp->rootdir, "/") == 0) { - rv = xbps_file_sha256_check(file, sha256d); - } else { - buf = xbps_xasprintf("%s/%s", xhp->rootdir, file); - rv = xbps_file_sha256_check(buf, sha256d); - free(buf); - } - if (rv == 0) - return 0; /* matched */ - else if (rv == ERANGE || rv == ENOENT) - return 1; /* no match */ - else - return -1; /* error */ -} From 62c82bc8eef81946a7dc865870bcf126cd90089d Mon Sep 17 00:00:00 2001 From: Duncan Overbruck Date: Thu, 17 Dec 2020 22:51:02 +0100 Subject: [PATCH 2/3] lib/transaction_files.c: also record new files from reinstall/downgrade --- lib/transaction_files.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/transaction_files.c b/lib/transaction_files.c index d45f4e3d7..fca183968 100644 --- a/lib/transaction_files.c +++ b/lib/transaction_files.c @@ -780,7 +780,8 @@ xbps_transaction_files(struct xbps_handle *xhp, xbps_object_iterator_t iter) update = (ttype == XBPS_TRANS_UPDATE); - if (ttype == XBPS_TRANS_INSTALL || ttype == XBPS_TRANS_UPDATE) { + if (ttype == XBPS_TRANS_INSTALL || ttype == XBPS_TRANS_UPDATE + || ttype == XBPS_TRANS_REINSTALL) { xbps_set_cb_state(xhp, XBPS_STATE_FILES, 0, pkgver, "%s: collecting files...", pkgver); rv = collect_binpkg_files(xhp, obj, idx, update); From c4c5114ceecb247f76181b21bed4e7bffe3fad46 Mon Sep 17 00:00:00 2001 From: Duncan Overbruck Date: Thu, 17 Dec 2020 22:51:49 +0100 Subject: [PATCH 3/3] include/xbps_api_impl.h: remove attributes from prototype --- include/xbps_api_impl.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/xbps_api_impl.h b/include/xbps_api_impl.h index fe10b0227..fdbbdcae8 100644 --- a/include/xbps_api_impl.h +++ b/include/xbps_api_impl.h @@ -160,8 +160,7 @@ int HIDDEN xbps_transaction_fetch(struct xbps_handle *, xbps_object_iterator_t); int HIDDEN xbps_transaction_pkg_deps(struct xbps_handle *, xbps_array_t, xbps_dictionary_t); -struct transaction_file HIDDEN * -xbps_transaction_file_new(struct xbps_handle *xhp UNUSED, const char *path); +struct transaction_file *xbps_transaction_file_new(struct xbps_handle *, const char *); char HIDDEN *xbps_get_remote_repo_string(const char *); int HIDDEN xbps_repo_sync(struct xbps_handle *, const char *);