From 61f0141a71d127114e4004552cd6c49c5a2cd72b Mon Sep 17 00:00:00 2001 From: Duncan Overbruck Date: Mon, 19 May 2025 18:27:58 +0200 Subject: [PATCH] lib: fix virtual package lookup performance Instead of having a dict with pkgvers as keys and iterating over all keys to match virtual packages, change the map to have pkgnames as keys, which hold another dict with all the old keys and values for a given package name. This significantly speeds up XBPS, since for every package it would iterate over the whole virtual package map. With recent changes to xbps-src, where it generates cmd: and py3: virtual packages, the number of virtual packages in the map has been growing a lot. --- include/xbps_api_impl.h | 2 +- lib/conf.c | 59 +++++++++++++-- lib/pkgdb.c | 96 ++++++++++++++++++------ lib/plist_find.c | 161 +++++++++++++++++++++++++++------------- 4 files changed, 238 insertions(+), 80 deletions(-) diff --git a/include/xbps_api_impl.h b/include/xbps_api_impl.h index 51b1481b9..32931fa64 100644 --- a/include/xbps_api_impl.h +++ b/include/xbps_api_impl.h @@ -113,7 +113,7 @@ int HIDDEN xbps_register_pkg(struct xbps_handle *, xbps_dictionary_t); char HIDDEN *xbps_archive_get_file(struct archive *, struct archive_entry *); xbps_dictionary_t HIDDEN xbps_archive_get_dictionary(struct archive *, struct archive_entry *); -const char HIDDEN *vpkg_user_conf(struct xbps_handle *, const char *, bool); +const char HIDDEN *vpkg_user_conf(struct xbps_handle *, const char *); xbps_array_t HIDDEN xbps_get_pkg_fulldeptree(struct xbps_handle *, const char *, bool); struct xbps_repo HIDDEN *xbps_regget_repo(struct xbps_handle *, diff --git a/lib/conf.c b/lib/conf.c index f9724db81..7d85d18b5 100644 --- a/lib/conf.c +++ b/lib/conf.c @@ -51,10 +51,47 @@ * Functions for parsing xbps configuration files. */ +static int +vpkg_map_add(xbps_dictionary_t d, const char *pkgname, const char *vpkgver, const char *provider) +{ + xbps_dictionary_t providers; + bool alloc = false; + int r; + + providers = xbps_dictionary_get(d, pkgname); + if (!providers) { + providers = xbps_dictionary_create(); + if (!providers) + return -ENOMEM; + alloc = true; + if (!xbps_dictionary_set(d, pkgname, providers)) { + r = errno ? -errno : -ENOMEM; + goto err; + } + } + + if (!xbps_dictionary_set_cstring(providers, vpkgver, provider)) { + r = errno ? -errno : -ENOMEM; + goto err; + } + + r = 0; +err: + if (alloc) + xbps_object_release(providers); + return r; +} + static int store_virtualpkg(struct xbps_handle *xhp, const char *path, size_t line, char *val) { + char namebuf[XBPS_NAME_SIZE]; + char pkgverbuf[XBPS_NAME_SIZE + sizeof("-99999_1")]; + const char *vpkgname, *vpkgver, *provider; char *p; + int r; + + /* * Parse strings delimited by ':' i.e * : @@ -66,13 +103,25 @@ store_virtualpkg(struct xbps_handle *xhp, const char *path, size_t line, char *v return 0; } *p++ = '\0'; + provider = p; + + if (xbps_pkg_name(namebuf, sizeof(namebuf), val)) { + vpkgname = namebuf; + vpkgver = val; + } else { + vpkgname = val; + snprintf(pkgverbuf, sizeof(pkgverbuf), "%s-99999_1", vpkgname); + vpkgver = pkgverbuf; + } - if (!xbps_dictionary_set_cstring(xhp->vpkgd, val, p)) - return -errno; - if (!xbps_dictionary_set_cstring(xhp->vpkgd_conf, val, p)) - return -errno; + r = vpkg_map_add(xhp->vpkgd, vpkgname, vpkgver, provider); + if (r < 0) + return r; + r = vpkg_map_add(xhp->vpkgd_conf, vpkgname, vpkgver, provider); + if (r < 0) + return r; xbps_dbg_printf("%s: added virtualpkg %s for %s\n", path, val, p); - return 1; + return 0; } static void diff --git a/lib/pkgdb.c b/lib/pkgdb.c index 8c0efa8fe..47c4b0ab3 100644 --- a/lib/pkgdb.c +++ b/lib/pkgdb.c @@ -35,6 +35,7 @@ #include #include +#include "xbps.h" #include "xbps_api_impl.h" /** @@ -145,26 +146,35 @@ pkgdb_map_vpkgs(struct xbps_handle *xhp) { xbps_object_iterator_t iter; xbps_object_t obj; - int rv = 0; + int r = 0; if (!xbps_dictionary_count(xhp->pkgdb)) return 0; if (xhp->vpkgd == NULL) { xhp->vpkgd = xbps_dictionary_create(); - assert(xhp->vpkgd); + if (!xhp->vpkgd) { + r = -errno; + xbps_error_printf("failed to create dictionary\n"); + return r; + } } + /* * This maps all pkgs that have virtualpkgs in pkgdb. */ iter = xbps_dictionary_iterator(xhp->pkgdb); - assert(iter); + if (!iter) { + r = -errno; + xbps_error_printf("failed to create iterator"); + goto out; + } while ((obj = xbps_object_iterator_next(iter))) { xbps_array_t provides; xbps_dictionary_t pkgd; const char *pkgver = NULL; - char pkgname[XBPS_NAME_SIZE] = {0}; + const char *pkgname = NULL; unsigned int cnt; pkgd = xbps_dictionary_get_keysym(xhp->pkgdb, obj); @@ -174,26 +184,53 @@ pkgdb_map_vpkgs(struct xbps_handle *xhp) continue; xbps_dictionary_get_cstring_nocopy(pkgd, "pkgver", &pkgver); - if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver)) { - rv = EINVAL; - goto out; - } + xbps_dictionary_get_cstring_nocopy(pkgd, "pkgname", &pkgname); + assert(pkgname); + for (unsigned int i = 0; i < cnt; i++) { + char vpkgname[XBPS_NAME_SIZE]; const char *vpkg = NULL; + xbps_dictionary_t providers; + bool alloc = false; xbps_array_get_cstring_nocopy(provides, i, &vpkg); - if (!xbps_dictionary_set_cstring(xhp->vpkgd, vpkg, pkgname)) { - xbps_dbg_printf("%s: set_cstring vpkg " - "%s pkgname %s\n", __func__, vpkg, pkgname); - rv = EINVAL; + if (!xbps_pkg_name(vpkgname, sizeof(vpkgname), vpkg)) { + xbps_warn_printf("%s: invalid provides: %s\n", pkgver, vpkg); + continue; + } + + providers = xbps_dictionary_get(xhp->vpkgd, vpkgname); + if (!providers) { + providers = xbps_dictionary_create(); + if (!providers) { + r = -errno; + xbps_error_printf("failed to create dictionary\n"); + goto out; + } + if (!xbps_dictionary_set(xhp->vpkgd, vpkgname, providers)) { + r = -errno; + xbps_error_printf("failed to set dictionary entry\n"); + xbps_object_release(providers); + goto out; + } + alloc = true; + } + + if (!xbps_dictionary_set_cstring(providers, vpkg, pkgname)) { + r = -errno; + xbps_error_printf("failed to set dictionary entry\n"); + if (alloc) + xbps_object_release(providers); goto out; } + if (alloc) + xbps_object_release(providers); xbps_dbg_printf("[pkgdb] added vpkg %s for %s\n", vpkg, pkgname); } } out: xbps_object_iterator_release(iter); - return rv; + return r; } static int @@ -398,6 +435,7 @@ generate_full_revdeps_tree(struct xbps_handle *xhp) { xbps_object_t obj; xbps_object_iterator_t iter; + xbps_dictionary_t vpkg_cache; if (xhp->pkgdb_revdeps) return; @@ -405,6 +443,9 @@ generate_full_revdeps_tree(struct xbps_handle *xhp) xhp->pkgdb_revdeps = xbps_dictionary_create(); assert(xhp->pkgdb_revdeps); + vpkg_cache = xbps_dictionary_create(); + assert(vpkg_cache); + iter = xbps_dictionary_iterator(xhp->pkgdb); assert(iter); @@ -421,8 +462,8 @@ generate_full_revdeps_tree(struct xbps_handle *xhp) xbps_dictionary_get_cstring_nocopy(pkgd, "pkgver", &pkgver); for (unsigned int i = 0; i < xbps_array_count(rundeps); i++) { xbps_array_t pkg; - const char *pkgdep = NULL, *vpkgname = NULL; - char *v, curpkgname[XBPS_NAME_SIZE]; + const char *pkgdep = NULL, *v; + char curpkgname[XBPS_NAME_SIZE]; bool alloc = false; xbps_array_get_cstring_nocopy(rundeps, i, &pkgdep); @@ -430,11 +471,24 @@ generate_full_revdeps_tree(struct xbps_handle *xhp) (!xbps_pkg_name(curpkgname, sizeof(curpkgname), pkgdep))) { abort(); } - vpkgname = vpkg_user_conf(xhp, curpkgname, false); - if (vpkgname == NULL) { - v = strdup(curpkgname); - } else { - v = strdup(vpkgname); + + /* TODO: this is kind of a workaround, to avoid calling vpkg_user_conf + * over and over again for the same packages which is slow. A better + * solution for itself vpkg_user_conf being slow should probably be + * implemented at some point. + */ + if (!xbps_dictionary_get_cstring_nocopy(vpkg_cache, curpkgname, &v)) { + const char *vpkgname = vpkg_user_conf(xhp, curpkgname); + if (vpkgname) { + v = vpkgname; + } else { + v = curpkgname; + } + errno = 0; + if (!xbps_dictionary_set_cstring_nocopy(vpkg_cache, curpkgname, v)) { + xbps_error_printf("%s\n", strerror(errno ? errno : ENOMEM)); + abort(); + } } pkg = xbps_dictionary_get(xhp->pkgdb_revdeps, v); @@ -446,12 +500,12 @@ generate_full_revdeps_tree(struct xbps_handle *xhp) xbps_array_add_cstring_nocopy(pkg, pkgver); xbps_dictionary_set(xhp->pkgdb_revdeps, v, pkg); } - free(v); if (alloc) xbps_object_release(pkg); } } xbps_object_iterator_release(iter); + xbps_object_release(vpkg_cache); } xbps_array_t diff --git a/lib/plist_find.c b/lib/plist_find.c index 4d51d1c0e..1ffd7608e 100644 --- a/lib/plist_find.c +++ b/lib/plist_find.c @@ -110,7 +110,7 @@ xbps_find_pkg_in_array(xbps_array_t a, const char *s, xbps_trans_type_t tt) } xbps_dictionary_t HIDDEN -xbps_find_virtualpkg_in_array(struct xbps_handle *x, +xbps_find_virtualpkg_in_array(struct xbps_handle *xhp, xbps_array_t a, const char *s, xbps_trans_type_t tt) @@ -118,11 +118,11 @@ xbps_find_virtualpkg_in_array(struct xbps_handle *x, xbps_dictionary_t pkgd; const char *vpkg; - assert(x); + assert(xhp); assert(xbps_object_type(a) == XBPS_TYPE_ARRAY); assert(s); - if ((vpkg = vpkg_user_conf(x, s, false))) { + if ((vpkg = vpkg_user_conf(xhp, s))) { if ((pkgd = get_pkg_in_array(a, vpkg, tt, true))) return pkgd; } @@ -188,82 +188,78 @@ match_pkg_by_pattern(xbps_dictionary_t repod, const char *p) } const char HIDDEN * -vpkg_user_conf(struct xbps_handle *xhp, const char *vpkg, bool only_conf) +vpkg_user_conf(struct xbps_handle *xhp, const char *vpkg) { - xbps_dictionary_t d; + char namebuf[XBPS_NAME_SIZE]; + xbps_dictionary_t providers; xbps_object_t obj; xbps_object_iterator_t iter; const char *pkg = NULL; + const char *pkgname; bool found = false; + enum { PKGPATTERN, PKGVER, PKGNAME } match; - assert(xhp); assert(vpkg); - if (only_conf) { - d = xhp->vpkgd_conf; + + if (xbps_pkgpattern_name(namebuf, sizeof(namebuf), vpkg)) { + match = PKGPATTERN; + pkgname = namebuf; + } else if (xbps_pkg_name(namebuf, sizeof(namebuf), vpkg)) { + match = PKGVER; + pkgname = namebuf; } else { - d = xhp->vpkgd; - /* init pkgdb just in case to detect vpkgs */ - (void)xbps_pkgdb_init(xhp); + match = PKGNAME; + pkgname = vpkg; } - if (d == NULL) + providers = xbps_dictionary_get(xhp->vpkgd, pkgname); + if (!providers) return NULL; - iter = xbps_dictionary_iterator(d); + iter = xbps_dictionary_iterator(providers); assert(iter); while ((obj = xbps_object_iterator_next(iter))) { xbps_string_t rpkg; char buf[XBPS_NAME_SIZE] = {0}; - char *vpkgver = NULL, *vpkgname = NULL; - const char *vpkg_conf = NULL; + const char *vpkg_conf = NULL, *vpkgname = NULL; vpkg_conf = xbps_dictionary_keysym_cstring_nocopy(obj); - rpkg = xbps_dictionary_get_keysym(xhp->vpkgd, obj); + rpkg = xbps_dictionary_get_keysym(providers, obj); pkg = xbps_string_cstring_nocopy(rpkg); if (xbps_pkg_version(vpkg_conf)) { if (!xbps_pkg_name(buf, sizeof(buf), vpkg_conf)) { abort(); } - vpkgname = strdup(buf); + vpkgname = buf; } else { - vpkgname = strdup(vpkg_conf); + vpkgname = vpkg_conf; } - assert(vpkgname); - if (xbps_pkgpattern_version(vpkg)) { + switch (match) { + case PKGPATTERN: if (xbps_pkg_version(vpkg_conf)) { if (!xbps_pkgpattern_match(vpkg_conf, vpkg)) { - free(vpkgname); continue; } } else { - vpkgver = xbps_xasprintf("%s-999999_1", vpkg_conf); - if (!xbps_pkgpattern_match(vpkgver, vpkg)) { - free(vpkgver); - free(vpkgname); - continue; - } - free(vpkgver); - } - } else if (xbps_pkg_version(vpkg)) { - if (!xbps_pkg_name(buf, sizeof(buf), vpkg)) { - abort(); + xbps_warn_printf("invalid: %s\n", vpkg_conf); } - if (strcmp(buf, vpkgname)) { - free(vpkgname); + break; + case PKGVER: + if (strcmp(buf, vpkgname) != 0) { continue; } - } else { - if (strcmp(vpkg, vpkgname)) { - free(vpkgname); + break; + case PKGNAME: + if (strcmp(vpkg, vpkgname) != 0) { continue; } + break; } xbps_dbg_printf("%s: vpkg_conf %s pkg %s vpkgname %s\n", __func__, vpkg_conf, pkg, vpkgname); - free(vpkgname); found = true; break; } @@ -277,24 +273,82 @@ xbps_find_virtualpkg_in_conf(struct xbps_handle *xhp, xbps_dictionary_t d, const char *pkg) { - xbps_dictionary_t pkgd; - const char *vpkg; + xbps_object_iterator_t iter; + xbps_object_t obj; + xbps_dictionary_t providers; + xbps_dictionary_t pkgd = NULL; + const char *cur; - /* Try matching vpkg from configuration files */ - vpkg = vpkg_user_conf(xhp, pkg, true); - if (vpkg != NULL) { - if (xbps_pkgpattern_version(vpkg)) - pkgd = match_pkg_by_pattern(d, vpkg); - else if (xbps_pkg_version(vpkg)) - pkgd = match_pkg_by_pkgver(d, vpkg); - else - pkgd = xbps_dictionary_get(d, vpkg); + if (!xhp->vpkgd_conf) + return NULL; - if (pkgd) - return pkgd; + providers = xbps_dictionary_get(xhp->vpkgd_conf, pkg); + if (!providers) + return NULL; + + iter = xbps_dictionary_iterator(providers); + assert(iter); + + while ((obj = xbps_object_iterator_next(iter))) { + xbps_string_t rpkg; + char buf[XBPS_NAME_SIZE] = {0}; + const char *vpkg_conf = NULL, *vpkgname = NULL; + + vpkg_conf = xbps_dictionary_keysym_cstring_nocopy(obj); + rpkg = xbps_dictionary_get_keysym(providers, obj); + cur = xbps_string_cstring_nocopy(rpkg); + assert(cur); + if (xbps_pkg_version(vpkg_conf)) { + if (!xbps_pkg_name(buf, sizeof(buf), vpkg_conf)) { + abort(); + } + vpkgname = buf; + } else { + vpkgname = vpkg_conf; + } + + if (xbps_pkgpattern_version(pkg)) { + if (xbps_pkg_version(vpkg_conf)) { + if (!xbps_pkgpattern_match(vpkg_conf, pkg)) { + continue; + } + } else { + char *vpkgver = xbps_xasprintf("%s-999999_1", vpkg_conf); + if (!xbps_pkgpattern_match(vpkgver, pkg)) { + free(vpkgver); + continue; + } + free(vpkgver); + } + } else if (xbps_pkg_version(pkg)) { + // XXX: this is the old behaviour of only matching pkgname's, + // this is kinda wrong when compared to matching patterns + // where all variants are tried. + if (!xbps_pkg_name(buf, sizeof(buf), pkg)) { + abort(); + } + if (strcmp(buf, vpkgname)) { + continue; + } + } else { + if (strcmp(pkg, vpkgname)) { + continue; + } + } + xbps_dbg_printf("%s: found: %s %s %s\n", __func__, vpkg_conf, cur, vpkgname); + + /* Try matching vpkg from configuration files */ + if (xbps_pkgpattern_version(cur)) + pkgd = match_pkg_by_pattern(d, cur); + else if (xbps_pkg_version(cur)) + pkgd = match_pkg_by_pkgver(d, cur); + else + pkgd = xbps_dictionary_get(d, cur); + break; } + xbps_object_iterator_release(iter); - return NULL; + return pkgd; } xbps_dictionary_t HIDDEN @@ -307,8 +361,9 @@ xbps_find_virtualpkg_in_dict(struct xbps_handle *xhp, xbps_dictionary_t pkgd = NULL; const char *vpkg; + // XXX: this is bad, dict != pkgdb, /* Try matching vpkg via xhp->vpkgd */ - vpkg = vpkg_user_conf(xhp, pkg, false); + vpkg = vpkg_user_conf(xhp, pkg); if (vpkg != NULL) { if (xbps_pkgpattern_version(vpkg)) pkgd = match_pkg_by_pattern(d, vpkg);