xbps_transaction_*: multiple performance improvements (v2).

This commit implements multiple performance improvements to the transaction code: - Don't process xbps_pkg_name() N times each time we access its package dictionary (via pkgdb or rpool), just do it once at xbps_pkgdb_init() time. At pkgdb init time, it just creates a property in pkgdb, "pkgname". At rpool time, each time a package is accessed, the "pkgname" string property is added. - The package transaction dictionary contains the "transaction" object to know what's the pkg type. This has been changed to an uint8, this simplifies the logic and it's faster than checking a string object. See xbps_trans_type_t and xbps_transaction_pkg_type(). - Fixed the issue that was marked with XXX in transaction shlibs checking code. This has been fixed and improved and resources are now just freed as expected. - Simplified random code all over the place, avoiding unnecessary allocations or operations. - Rename some transaction files to have a better description. This is my first rototill to the code in 2020.
2020-02-21 09:08:22 +01:00
parent 701132071d
commit 06c9891ae3
28 changed files with 960 additions and 742 deletions
--- a/bin/xbps-install/state_cb.c
+++ b/bin/xbps-install/state_cb.c
@@ -66,7 +66,7 @@ state_cb(const struct xbps_state_cb_data *xscd, void *cbdata UNUSED)
 		printf("[*] pkgdb upgrade in progress, please wait...\n");
 		break;
 	case XBPS_STATE_REPOSYNC:
-		printf("[*] Updating `%s' ...\n", xscd->arg);
+		printf("[*] Updating repository `%s' ...\n", xscd->arg);
 		break;
 	case XBPS_STATE_TRANS_ADDPKG:
 		if (xscd->xhp->flags & XBPS_FLAG_VERBOSE)
--- a/bin/xbps-install/transaction.c
+++ b/bin/xbps-install/transaction.c
@@ -47,6 +47,38 @@ print_array(xbps_array_t a)
 	}
 }

+static const char *
+ttype2str(xbps_dictionary_t pkg_repod)
+{
+	uint8_t r;
+
+	assert(pkg_repod);
+
+	if (!xbps_dictionary_get_uint8(pkg_repod, "transaction", &r))
+		return NULL;
+
+	switch (r) {
+	case XBPS_TRANS_INSTALL:
+		return "install";
+	case XBPS_TRANS_REINSTALL:
+		return "reinstall";
+	case XBPS_TRANS_UPDATE:
+		return "update";
+	case XBPS_TRANS_REMOVE:
+		return "remove";
+	case XBPS_TRANS_CONFIGURE:
+		return "configure";
+	case XBPS_TRANS_HOLD:
+		return "hold";
+	case XBPS_TRANS_DOWNLOAD:
+		return "download";
+	default:
+		return "unknown";
+	}
+
+	return NULL;
+}
+
 static void
 show_actions(xbps_object_iterator_t iter)
 {
@@ -54,13 +86,12 @@ show_actions(xbps_object_iterator_t iter)
 	const char *repoloc, *trans, *pkgver, *arch;
 	uint64_t isize, dsize;

-	repoloc = trans = pkgver = arch = NULL;
-	isize = dsize = 0;
-
 	while ((obj = xbps_object_iterator_next(iter)) != NULL) {
-		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &trans);
+		repoloc = trans = pkgver = arch = NULL;
+		isize = dsize = 0;
+
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
-		printf("%s %s", pkgver, trans);
+		printf("%s %s", pkgver, ttype2str(obj));
 		xbps_dictionary_get_cstring_nocopy(obj, "repository", &repoloc);
 		xbps_dictionary_get_cstring_nocopy(obj, "architecture", &arch);
 		if (repoloc && arch)
@@ -77,28 +108,25 @@ show_actions(xbps_object_iterator_t iter)
 }

 static void
-show_package_list(struct transaction *trans, const char *match, int cols)
+show_package_list(struct transaction *trans, xbps_trans_type_t ttype, int cols)
 {
+	xbps_dictionary_t ipkgd;
 	xbps_object_t obj;
+	xbps_trans_type_t tt;
+	const char *pkgver, *pkgname, *ipkgver, *version, *iversion;
+	char *buf = NULL;

 	while ((obj = xbps_object_iterator_next(trans->iter)) != NULL) {
-		const char *pkgver, *tract;
-		char *buf = NULL;
 		bool dload = false;

+		pkgver = ipkgver = version = iversion = NULL;
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
-		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &tract);
+		xbps_dictionary_get_cstring_nocopy(obj, "pkgname", &pkgname);
 		xbps_dictionary_get_bool(obj, "download", &dload);
+		tt = xbps_transaction_pkg_type(obj);

-		if (match && strcmp(tract, "update") == 0) {
-			xbps_dictionary_t ipkgd;
-			const char *ipkgver, *iversion, *version;
-			char pkgname[XBPS_NAME_SIZE];
-
+		if (tt == XBPS_TRANS_UPDATE) {
 			/* get installed pkgver */
-			if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver)) {
-				abort();
-			}
 			ipkgd = xbps_pkgdb_get_pkg(trans->xhp, pkgname);
 			assert(ipkgd);
 			xbps_dictionary_get_cstring_nocopy(ipkgd, "pkgver", &ipkgver);
@@ -106,7 +134,7 @@ show_package_list(struct transaction *trans, const char *match, int cols)
 			iversion = xbps_pkg_version(ipkgver);
 			buf = xbps_xasprintf("%s (%s -> %s)", pkgname, iversion, version);
 		}
-		if ((match && (strcmp(match, tract) == 0)) || (!match && dload)) {
+		if ((ttype && (ttype == tt)) || (!ttype && dload)) {
 			if (buf) {
 				print_package_line(buf, cols, false);
 				free(buf);
@@ -135,23 +163,15 @@ show_transaction_sizes(struct transaction *trans, int cols)
 		if (trans->dl_pkgcnt) {
 			printf("%u package%s will be downloaded:\n",
 			    trans->dl_pkgcnt, trans->dl_pkgcnt == 1 ? "" : "s");
-			show_package_list(trans, NULL, cols);
+			show_package_list(trans, XBPS_TRANS_DOWNLOAD, cols);
 			printf("\n");
 		}
-		if (trans->xhp->flags & XBPS_FLAG_DOWNLOAD_ONLY) {
-			trans->inst_pkgcnt = 0;
-			trans->up_pkgcnt = 0;
-			trans->cf_pkgcnt = 0;
-			trans->rm_pkgcnt = 0;
-			goto out;
-		}
-
 		xbps_dictionary_get_uint32(trans->d, "total-install-pkgs",
 		    &trans->inst_pkgcnt);
 		if (trans->inst_pkgcnt) {
 			printf("%u package%s will be installed:\n",
 			    trans->inst_pkgcnt, trans->inst_pkgcnt == 1 ? "" : "s");
-			show_package_list(trans, "install", cols);
+			show_package_list(trans, XBPS_TRANS_INSTALL, cols);
 			printf("\n");
 		}
 		xbps_dictionary_get_uint32(trans->d, "total-update-pkgs",
@@ -159,7 +179,7 @@ show_transaction_sizes(struct transaction *trans, int cols)
 		if (trans->up_pkgcnt) {
 			printf("%u package%s will be updated:\n",
 			    trans->up_pkgcnt, trans->up_pkgcnt == 1 ? "" : "s");
-			show_package_list(trans, "update", cols);
+			show_package_list(trans, XBPS_TRANS_UPDATE, cols);
 			printf("\n");
 		}
 		xbps_dictionary_get_uint32(trans->d, "total-configure-pkgs",
@@ -167,7 +187,7 @@ show_transaction_sizes(struct transaction *trans, int cols)
 		if (trans->cf_pkgcnt) {
 			printf("%u package%s will be configured:\n",
 			    trans->cf_pkgcnt, trans->cf_pkgcnt == 1 ? "" : "s");
-			show_package_list(trans, "configure", cols);
+			show_package_list(trans, XBPS_TRANS_CONFIGURE, cols);
 			printf("\n");
 		}
 		xbps_dictionary_get_uint32(trans->d, "total-remove-pkgs",
@@ -175,11 +195,10 @@ show_transaction_sizes(struct transaction *trans, int cols)
 		if (trans->rm_pkgcnt) {
 			printf("%u package%s will be removed:\n",
 			    trans->rm_pkgcnt, trans->rm_pkgcnt == 1 ? "" : "s");
-			show_package_list(trans, "remove", cols);
+			show_package_list(trans, XBPS_TRANS_REMOVE, cols);
 			printf("\n");
 		}
 	}
-out:
 	/*
 	 * Show total download/installed/removed size for all required packages.
 	 */
@@ -232,18 +251,16 @@ static bool
 all_pkgs_on_hold(struct transaction *trans)
 {
 	xbps_object_t obj;
-	const char *action = NULL;
+	xbps_trans_type_t ttype;
 	bool all_on_hold = true;

 	while ((obj = xbps_object_iterator_next(trans->iter)) != NULL) {
-		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &action);
-		if (strcmp(action, "hold")) {
+		ttype = xbps_transaction_pkg_type(obj);
+		if (ttype != XBPS_TRANS_HOLD) {
 			all_on_hold = false;
-			goto out;
+			break;
 		}
 	}
-
-out:
 	xbps_object_iterator_reset(trans->iter);
 	return all_on_hold;
 }
--- a/bin/xbps-install/util.c
+++ b/bin/xbps-install/util.c
@@ -81,15 +81,12 @@ static unsigned int
 find_longest_pkgname(struct transaction *trans)
 {
 	xbps_object_t obj;
-	const char *pkgver;
-	char pkgname[XBPS_NAME_SIZE];
+	const char *pkgname;
 	unsigned int len = 0, max = 0;

 	while ((obj = xbps_object_iterator_next(trans->iter)) != NULL) {
-		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
-		if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver)) {
-			abort();
-		}
+		if (!xbps_dictionary_get_cstring_nocopy(obj, "pkgname", &pkgname))
+			continue;
 		len = strlen(pkgname);
 		if (max == 0 || len > max)
 			max = len;
@@ -101,9 +98,12 @@ find_longest_pkgname(struct transaction *trans)
 bool
 print_trans_colmode(struct transaction *trans, int cols)
 {
+	xbps_dictionary_t ipkgd;
 	xbps_object_t obj;
-	uint64_t dlsize = 0;
+	xbps_trans_type_t ttype;
+	const char *pkgver, *pkgname, *ipkgver, *ver, *iver, *tract;
 	char size[8];
+	uint64_t dlsize = 0;
 	unsigned int x, blen, pnamelen;
 	int hdrlen;

@@ -123,43 +123,49 @@ print_trans_colmode(struct transaction *trans, int cols)
 	printf("Action    Version           New version            Download size\n");

 	while ((obj = xbps_object_iterator_next(trans->iter)) != NULL) {
-		xbps_dictionary_t ipkgd;
-		const char *pkgver, *ipkgver, *ver, *iver, *tract;
-		char pkgname[XBPS_NAME_SIZE];
 		bool dload = false;

-		ver = iver = NULL;
-
+		pkgver = pkgname = ipkgver = ver = iver = NULL;
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
-		xbps_dictionary_get_cstring_nocopy(obj, "transaction", &tract);
+		xbps_dictionary_get_cstring_nocopy(obj, "pkgname", &pkgname);
 		xbps_dictionary_get_uint64(obj, "filename-size", &dlsize);
 		xbps_dictionary_get_bool(obj, "download", &dload);

-		if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver)) {
-			abort();
-		}
+		ttype = xbps_transaction_pkg_type(obj);

+		tract = "unknown";
 		if (trans->xhp->flags & XBPS_FLAG_DOWNLOAD_ONLY) {
 			tract = "download";
 		}

-		if (strcmp(tract, "install") == 0) {
+		if (ttype == XBPS_TRANS_INSTALL) {
 			trans->inst_pkgcnt++;
-		} else if (strcmp(tract, "update") == 0) {
+			tract = "install";
+		} else if (ttype == XBPS_TRANS_REINSTALL) {
+			trans->inst_pkgcnt++;
+			tract = "reinstall";
+		} else if (ttype == XBPS_TRANS_UPDATE) {
 			trans->up_pkgcnt++;
-		} else if (strcmp(tract, "remove") == 0) {
+			tract = "update";
+		} else if (ttype == XBPS_TRANS_REMOVE) {
 			trans->rm_pkgcnt++;
-		} else if (strcmp(tract, "configure") == 0) {
+			tract = "remove";
+		} else if (ttype == XBPS_TRANS_CONFIGURE) {
 			trans->cf_pkgcnt++;
-		}
-		ipkgd = xbps_pkgdb_get_pkg(trans->xhp, pkgname);
-
-		if (trans->xhp->flags & XBPS_FLAG_DOWNLOAD_ONLY) {
-			ipkgd = NULL;
+			tract = "configure";
+		} else if (ttype == XBPS_TRANS_HOLD) {
+			tract = "hold";
+		} else if (ttype == XBPS_TRANS_DOWNLOAD) {
+			tract = "download";
 		}
 		if (dload) {
 			trans->dl_pkgcnt++;
 		}
+
+		ipkgd = xbps_pkgdb_get_pkg(trans->xhp, pkgname);
+		if (trans->xhp->flags & XBPS_FLAG_DOWNLOAD_ONLY) {
+			ipkgd = NULL;
+		}
 		if (ipkgd) {
 			xbps_dictionary_get_cstring_nocopy(ipkgd, "pkgver", &ipkgver);
 			iver = xbps_pkg_version(ipkgver);
@@ -167,10 +173,8 @@ print_trans_colmode(struct transaction *trans, int cols)
 		ver = xbps_pkg_version(pkgver);
 		if (iver) {
 			int rv = xbps_cmpver(iver, ver);
-			if (rv == 1 && strcmp(tract, "hold"))
+			if (rv == 1 && ttype != XBPS_TRANS_HOLD)
 				tract = "downgrade";
-			else if (rv == 0 && strcmp(tract, "remove"))
-				tract = "reinstall";
 		}
 		/* print pkgname and some blanks */
 		blen = pnamelen - strlen(pkgname);
@@ -192,7 +196,7 @@ print_trans_colmode(struct transaction *trans, int cols)
 		for (x = strlen(iver); x < 17; x++)
 			printf(" ");

-		if (strcmp(tract, "remove") == 0) {
+		if (ttype == XBPS_TRANS_REMOVE) {
 			ver = "-";
 		}
 		if (dload)