[pacman-dev] [PATCH 2/3] Reduce calls to getcols

Dan McGee dan at archlinux.org
Fri Mar 16 19:19:58 EDT 2012


This dramatically improves upon a much older attempt in 2008 in commit
ce3d70aa99ab86. We don't need to call it once per line we print unless
there is a reasonable expectation of being able to resize the terminal
mid-operation; this is really only the case during our callback progress
bars.

Some before and after numbers of ioctl() calls, gleaned from strace of
the following operations (no targets to any of them to maximize the
amount of output):

    pacman -Qii :  37768 ->  2616  (93.1% decrease)
    pacman -Qs  :   2616 ->     4  (99.8%)
    pacman -Sii : 133036 -> 10926  (91.8%)
    pacman -Ss  :  10926 ->    14  (99.9%)

Obviously the search results are astounding; we only call getcols()
once in the case of -Qs, and once per repo in the case of -Ss. For
-Qii and -Sii we are still calling it once per package, but this is
much better than once per line of info output.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 src/pacman/callback.c |    2 +-
 src/pacman/package.c  |   67 +++++++++++++++++++++++++------------------------
 src/pacman/pacman.c   |    2 +-
 src/pacman/query.c    |    8 +++---
 src/pacman/sync.c     |   10 +++++---
 src/pacman/util.c     |   63 +++++++++++++++++++++++++---------------------
 src/pacman/util.h     |   16 +++++++-----
 7 files changed, 92 insertions(+), 76 deletions(-)

diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index 61f517c..01c6b61 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -308,7 +308,7 @@ void cb_question(alpm_question_t event, void *data1, void *data2,
 							":: The following package cannot be upgraded due to unresolvable dependencies:\n",
 							":: The following packages cannot be upgraded due to unresolvable dependencies:\n",
 							count));
-				list_display("     ", namelist);
+				list_display("     ", namelist, getcols(fileno(stdout)));
 				printf("\n");
 				*response = noyes(_n(
 							"Do you want to skip the above package for this upgrade?",
diff --git a/src/pacman/package.c b/src/pacman/package.c
index 6e091d5..942797a 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -40,14 +40,14 @@
  * @param deps a list with items of type alpm_depend_t
  */
 static void deplist_display(const char *title,
-		alpm_list_t *deps)
+		alpm_list_t *deps, unsigned short cols)
 {
 	alpm_list_t *i, *text = NULL;
 	for(i = deps; i; i = alpm_list_next(i)) {
 		alpm_depend_t *dep = i->data;
 		text = alpm_list_add(text, alpm_dep_compute_string(dep));
 	}
-	list_display(title, text);
+	list_display(title, text, cols);
 	FREELIST(text);
 }
 
@@ -55,14 +55,14 @@ static void deplist_display(const char *title,
  * @param optdeps a list with items of type alpm_optdepend_t
  */
 static void optdeplist_display(const char *title,
-		alpm_list_t *optdeps)
+		alpm_list_t *optdeps, unsigned short cols)
 {
 	alpm_list_t *i, *text = NULL;
 	for(i = optdeps; i; i = alpm_list_next(i)) {
 		alpm_depend_t *optdep = i->data;
 		text = alpm_list_add(text, alpm_dep_compute_string(optdep));
 	}
-	list_display_linebreak(title, text);
+	list_display_linebreak(title, text, cols);
 	FREELIST(text);
 }
 
@@ -76,14 +76,13 @@ static void optdeplist_display(const char *title,
  */
 void dump_pkg_full(alpm_pkg_t *pkg, int extra)
 {
-	const char *reason;
-	alpm_list_t *validation = NULL;
+	unsigned short cols;
 	time_t bdate, idate;
-	char bdatestr[50] = "", idatestr[50] = "";
-	const char *label;
-	double size;
-	alpm_list_t *requiredby = NULL;
 	alpm_pkgfrom_t from;
+	double size;
+	char bdatestr[50] = "", idatestr[50] = "";
+	const char *label, *reason;
+	alpm_list_t *validation = NULL, *requiredby = NULL;
 
 	from = alpm_pkg_get_origin(pkg);
 
@@ -133,24 +132,26 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)
 		requiredby = alpm_pkg_compute_requiredby(pkg);
 	}
 
+	cols = getcols(fileno(stdout));
+
 	/* actual output */
 	if(from == PKG_FROM_SYNCDB) {
 		string_display(_("Repository     :"),
-				alpm_db_get_name(alpm_pkg_get_db(pkg)));
+				alpm_db_get_name(alpm_pkg_get_db(pkg)), cols);
 	}
-	string_display(_("Name           :"), alpm_pkg_get_name(pkg));
-	string_display(_("Version        :"), alpm_pkg_get_version(pkg));
-	string_display(_("URL            :"), alpm_pkg_get_url(pkg));
-	list_display(_("Licenses       :"), alpm_pkg_get_licenses(pkg));
-	list_display(_("Groups         :"), alpm_pkg_get_groups(pkg));
-	deplist_display(_("Provides       :"), alpm_pkg_get_provides(pkg));
-	deplist_display(_("Depends On     :"), alpm_pkg_get_depends(pkg));
-	optdeplist_display(_("Optional Deps  :"), alpm_pkg_get_optdepends(pkg));
+	string_display(_("Name           :"), alpm_pkg_get_name(pkg), cols);
+	string_display(_("Version        :"), alpm_pkg_get_version(pkg), cols);
+	string_display(_("URL            :"), alpm_pkg_get_url(pkg), cols);
+	list_display(_("Licenses       :"), alpm_pkg_get_licenses(pkg), cols);
+	list_display(_("Groups         :"), alpm_pkg_get_groups(pkg), cols);
+	deplist_display(_("Provides       :"), alpm_pkg_get_provides(pkg), cols);
+	deplist_display(_("Depends On     :"), alpm_pkg_get_depends(pkg), cols);
+	optdeplist_display(_("Optional Deps  :"), alpm_pkg_get_optdepends(pkg), cols);
 	if(extra || from == PKG_FROM_LOCALDB) {
-		list_display(_("Required By    :"), requiredby);
+		list_display(_("Required By    :"), requiredby, cols);
 	}
-	deplist_display(_("Conflicts With :"), alpm_pkg_get_conflicts(pkg));
-	deplist_display(_("Replaces       :"), alpm_pkg_get_replaces(pkg));
+	deplist_display(_("Conflicts With :"), alpm_pkg_get_conflicts(pkg), cols);
+	deplist_display(_("Replaces       :"), alpm_pkg_get_replaces(pkg), cols);
 
 	size = humanize_size(alpm_pkg_get_size(pkg), 'K', 2, &label);
 	if(from == PKG_FROM_SYNCDB) {
@@ -162,35 +163,35 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)
 	size = humanize_size(alpm_pkg_get_isize(pkg), 'K', 2, &label);
 	printf(_("Installed Size : %6.2f %s\n"), size, label);
 
-	string_display(_("Packager       :"), alpm_pkg_get_packager(pkg));
-	string_display(_("Architecture   :"), alpm_pkg_get_arch(pkg));
-	string_display(_("Build Date     :"), bdatestr);
+	string_display(_("Packager       :"), alpm_pkg_get_packager(pkg), cols);
+	string_display(_("Architecture   :"), alpm_pkg_get_arch(pkg), cols);
+	string_display(_("Build Date     :"), bdatestr, cols);
 	if(from == PKG_FROM_LOCALDB) {
-		string_display(_("Install Date   :"), idatestr);
-		string_display(_("Install Reason :"), reason);
+		string_display(_("Install Date   :"), idatestr, cols);
+		string_display(_("Install Reason :"), reason, cols);
 	}
 	if(from == PKG_FROM_FILE || from == PKG_FROM_LOCALDB) {
 		string_display(_("Install Script :"),
-				alpm_pkg_has_scriptlet(pkg) ?  _("Yes") : _("No"));
+				alpm_pkg_has_scriptlet(pkg) ?  _("Yes") : _("No"), cols);
 	}
 
-	list_display(_("Validated By   :"), validation);
+	list_display(_("Validated By   :"), validation, cols);
 
 	if(from == PKG_FROM_FILE) {
 		alpm_siglist_t siglist;
 		int err = alpm_pkg_check_pgp_signature(pkg, &siglist);
 		if(err && alpm_errno(config->handle) == ALPM_ERR_SIG_MISSING) {
-			string_display(_("Signatures     :"), _("None"));
+			string_display(_("Signatures     :"), _("None"), cols);
 		} else if(err) {
 			string_display(_("Signatures     :"),
-					alpm_strerror(alpm_errno(config->handle)));
+					alpm_strerror(alpm_errno(config->handle)), cols);
 		} else {
-			signature_display(_("Signatures     :"), &siglist);
+			signature_display(_("Signatures     :"), &siglist, cols);
 		}
 		alpm_siglist_cleanup(&siglist);
 	}
 
-	string_display(_("Description    :"), alpm_pkg_get_desc(pkg));
+	string_display(_("Description    :"), alpm_pkg_get_desc(pkg), cols);
 
 	/* Print additional package info if info flag passed more than once */
 	if(from == PKG_FROM_LOCALDB && extra) {
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 107aa18..1e6797c 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -897,7 +897,7 @@ int main(int argc, char *argv[])
 		printf("Lock File : %s\n", alpm_option_get_lockfile(config->handle));
 		printf("Log File  : %s\n", alpm_option_get_logfile(config->handle));
 		printf("GPG Dir   : %s\n", alpm_option_get_gpgdir(config->handle));
-		list_display("Targets   :", pm_targets);
+		list_display("Targets   :", pm_targets, 0);
 	}
 
 	/* Log command line */
diff --git a/src/pacman/query.c b/src/pacman/query.c
index 1d27232..a1cc1cf 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -246,6 +246,7 @@ static int query_search(alpm_list_t *targets)
 	alpm_list_t *i, *searchlist;
 	int freelist;
 	alpm_db_t *db_local = alpm_get_localdb(config->handle);
+	unsigned short cols;
 
 	/* if we have a targets list, search for packages matching it */
 	if(targets) {
@@ -259,6 +260,7 @@ static int query_search(alpm_list_t *targets)
 		return 1;
 	}
 
+	cols = getcols(fileno(stdout));
 	for(i = searchlist; i; i = alpm_list_next(i)) {
 		alpm_list_t *grp;
 		alpm_pkg_t *pkg = i->data;
@@ -286,10 +288,10 @@ static int query_search(alpm_list_t *targets)
 			}
 
 			/* we need a newline and initial indent first */
-			printf("\n    ");
-			indentprint(alpm_pkg_get_desc(pkg), 4);
+			fputs("\n    ", stdout);
+			indentprint(alpm_pkg_get_desc(pkg), 4, cols);
 		}
-		printf("\n");
+		fputc('\n', stdout);
 	}
 
 	/* we only want to free if the list was a search list */
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index d782a94..74e417c 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -360,6 +360,7 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)
 
 	for(i = syncs; i; i = alpm_list_next(i)) {
 		alpm_db_t *db = i->data;
+		unsigned short cols;
 		/* if we have a targets list, search for packages matching it */
 		if(targets) {
 			ret = alpm_db_search(db, targets);
@@ -373,6 +374,7 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)
 		} else {
 			found = 1;
 		}
+		cols = getcols(fileno(stdout));
 		for(j = ret; j; j = alpm_list_next(j)) {
 			alpm_list_t *grp;
 			alpm_pkg_t *pkg = j->data;
@@ -402,10 +404,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)
 				print_installed(db_local, pkg);
 
 				/* we need a newline and initial indent first */
-				printf("\n    ");
-				indentprint(alpm_pkg_get_desc(pkg), 4);
+				fputs("\n    ", stdout);
+				indentprint(alpm_pkg_get_desc(pkg), 4, cols);
 			}
-			printf("\n");
+			fputc('\n', stdout);
 		}
 		/* we only want to free if the list was a search list */
 		if(freelist) {
@@ -1005,7 +1007,7 @@ int pacman_sync(alpm_list_t *targets)
 			if(config->op_s_upgrade || (tmp = alpm_list_diff(targets, packages, (alpm_list_fn_cmp)strcmp))) {
 				alpm_list_free(tmp);
 				printf(_(":: The following packages should be upgraded first :\n"));
-				list_display("   ", packages);
+				list_display("   ", packages, getcols(fileno(stdout)));
 				if(yesno(_(":: Do you want to cancel the current operation\n"
 								":: and upgrade these packages now?"))) {
 					FREELIST(targs);
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 15cd6fd..4b8684e 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -254,12 +254,11 @@ char *mdirname(const char *path)
 
 /* output a string, but wrap words properly with a specified indentation
  */
-void indentprint(const char *str, size_t indent)
+void indentprint(const char *str, unsigned short indent, unsigned short cols)
 {
 	wchar_t *wcstr;
 	const wchar_t *p;
 	int len, cidx;
-	const unsigned short cols = getcols(fileno(stdout));
 
 	if(!str) {
 		return;
@@ -461,7 +460,7 @@ static size_t string_length(const char *s)
 	return len;
 }
 
-void string_display(const char *title, const char *string)
+void string_display(const char *title, const char *string, unsigned short cols)
 {
 	if(title) {
 		printf("%s ", title);
@@ -471,7 +470,7 @@ void string_display(const char *title, const char *string)
 	} else {
 		/* compute the length of title + a space */
 		size_t len = string_length(title) + 1;
-		indentprint(string, len);
+		indentprint(string, (unsigned short)len, cols);
 	}
 	printf("\n");
 }
@@ -603,11 +602,11 @@ static size_t table_calc_widths(const alpm_list_t *header,
  *               of headers
  * @param rows the rows to display as a list of lists of strings. the outer
  *             list represents the rows, the inner list the cells (= columns)
- *
+ * @param cols the number of columns available in the terminal
  * @return -1 if not enough terminal cols available, else 0
  */
 int table_display(const char *title, const alpm_list_t *header,
-		const alpm_list_t *rows)
+		const alpm_list_t *rows, unsigned short cols)
 {
 	const unsigned short padding = 2;
 	const alpm_list_t *i;
@@ -622,7 +621,7 @@ int table_display(const char *title, const alpm_list_t *header,
 	totalwidth = table_calc_widths(header, rows, padding, totalcols,
 			&widths, &has_data);
 	/* return -1 if terminal is not wide enough */
-	if(totalwidth > getcols(fileno(stdout))) {
+	if(totalwidth > cols) {
 		pm_printf(ALPM_LOG_WARNING,
 				_("insufficient columns available for table display\n"));
 		return -1;
@@ -647,7 +646,8 @@ int table_display(const char *title, const alpm_list_t *header,
 	return 0;
 }
 
-void list_display(const char *title, const alpm_list_t *list)
+void list_display(const char *title, const alpm_list_t *list,
+		unsigned short maxcols)
 {
 	const alpm_list_t *i;
 	size_t len = 0;
@@ -660,7 +660,6 @@ void list_display(const char *title, const alpm_list_t *list)
 	if(!list) {
 		printf("%s\n", _("None"));
 	} else {
-		const unsigned short maxcols = getcols(fileno(stdout));
 		size_t cols = len;
 		const char *str = list->data;
 		fputs(str, stdout);
@@ -688,12 +687,13 @@ void list_display(const char *title, const alpm_list_t *list)
 	}
 }
 
-void list_display_linebreak(const char *title, const alpm_list_t *list)
+void list_display_linebreak(const char *title, const alpm_list_t *list,
+		unsigned short maxcols)
 {
-	size_t len = 0;
+	unsigned short len = 0;
 
 	if(title) {
-		len = string_length(title) + 1;
+		len = (unsigned short)string_length(title) + 1;
 		printf("%s ", title);
 	}
 
@@ -702,7 +702,7 @@ void list_display_linebreak(const char *title, const alpm_list_t *list)
 	} else {
 		const alpm_list_t *i;
 		/* Print the first element */
-		indentprint((const char *)list->data, len);
+		indentprint((const char *)list->data, len, maxcols);
 		printf("\n");
 		/* Print the rest */
 		for(i = alpm_list_next(list); i; i = alpm_list_next(i)) {
@@ -710,18 +710,19 @@ void list_display_linebreak(const char *title, const alpm_list_t *list)
 			for(j = 1; j <= len; j++) {
 				printf(" ");
 			}
-			indentprint((const char *)i->data, len);
+			indentprint((const char *)i->data, len, maxcols);
 			printf("\n");
 		}
 	}
 }
 
-void signature_display(const char *title, alpm_siglist_t *siglist)
+void signature_display(const char *title, alpm_siglist_t *siglist,
+		unsigned short maxcols)
 {
-	size_t len = 0;
+	unsigned short len = 0;
 
 	if(title) {
-		len = string_length(title) + 1;
+		len = (unsigned short)string_length(title) + 1;
 		printf("%s ", title);
 	}
 	if(siglist->count == 0) {
@@ -785,7 +786,7 @@ void signature_display(const char *title, alpm_siglist_t *siglist)
 				pm_printf(ALPM_LOG_ERROR,  _("failed to allocate string\n"));
 				continue;
 			}
-			indentprint(sigline, len);
+			indentprint(sigline, len, maxcols);
 			printf("\n");
 			free(sigline);
 		}
@@ -869,6 +870,7 @@ static void _display_targets(alpm_list_t *targets, int verbose)
 	const char *label;
 	double size;
 	off_t isize = 0, rsize = 0, dlsize = 0;
+	unsigned short cols;
 	alpm_list_t *i, *rows = NULL, *names = NULL;
 
 	if(!targets) {
@@ -909,17 +911,18 @@ static void _display_targets(alpm_list_t *targets, int verbose)
 
 	/* print to screen */
 	pm_asprintf(&str, _("Targets (%d):"), alpm_list_count(targets));
-
 	printf("\n");
+
+	cols = getcols(fileno(stdout));
 	if(verbose) {
 		alpm_list_t *header = create_verbose_header();
-		if(table_display(str, header, rows) != 0) {
+		if(table_display(str, header, rows, cols) != 0) {
 			/* fallback to list display if table wouldn't fit */
-			list_display(str, names);
+			list_display(str, names, cols);
 		}
 		alpm_list_free(header);
 	} else {
-		list_display(str, names);
+		list_display(str, names, cols);
 	}
 	printf("\n");
 
@@ -1205,7 +1208,8 @@ void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg)
 
 	if(optstrings) {
 		printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg));
-		list_display_linebreak("   ", optstrings);
+		unsigned short cols = getcols(fileno(stdout));
+		list_display_linebreak("   ", optstrings, cols);
 	}
 
 	alpm_list_free(optdeps);
@@ -1226,19 +1230,21 @@ void display_optdepends(alpm_pkg_t *pkg)
 
 	if(optstrings) {
 		printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg));
-		list_display_linebreak("   ", optstrings);
+		unsigned short cols = getcols(fileno(stdout));
+		list_display_linebreak("   ", optstrings, cols);
 	}
 
 	FREELIST(optstrings);
 }
 
-static void display_repo_list(const char *dbname, alpm_list_t *list)
+static void display_repo_list(const char *dbname, alpm_list_t *list,
+		unsigned short cols)
 {
 	const char *prefix= "  ";
 
 	printf(":: ");
 	printf(_("Repository %s\n"), dbname);
-	list_display(prefix, list);
+	list_display(prefix, list, cols);
 }
 
 void select_display(const alpm_list_t *pkglist)
@@ -1248,6 +1254,7 @@ void select_display(const alpm_list_t *pkglist)
 	alpm_list_t *list = NULL;
 	char *string = NULL;
 	const char *dbname = NULL;
+	unsigned short cols = getcols(fileno(stdout));
 
 	for(i = pkglist; i; i = i->next) {
 		alpm_pkg_t *pkg = i->data;
@@ -1256,7 +1263,7 @@ void select_display(const alpm_list_t *pkglist)
 		if(!dbname)
 			dbname = alpm_db_get_name(db);
 		if(strcmp(alpm_db_get_name(db), dbname) != 0) {
-			display_repo_list(dbname, list);
+			display_repo_list(dbname, list, cols);
 			FREELIST(list);
 			dbname = alpm_db_get_name(db);
 		}
@@ -1265,7 +1272,7 @@ void select_display(const alpm_list_t *pkglist)
 		list = alpm_list_add(list, string);
 		nth++;
 	}
-	display_repo_list(dbname, list);
+	display_repo_list(dbname, list, cols);
 	FREELIST(list);
 }
 
diff --git a/src/pacman/util.h b/src/pacman/util.h
index fc97707..7773c81 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -51,17 +51,21 @@ unsigned short getcols(int fd);
 int rmrf(const char *path);
 const char *mbasename(const char *path);
 char *mdirname(const char *path);
-void indentprint(const char *str, size_t indent);
+void indentprint(const char *str, unsigned short indent, unsigned short cols);
 size_t strtrim(char *str);
 char *strreplace(const char *str, const char *needle, const char *replace);
 alpm_list_t *strsplit(const char *str, const char splitchar);
-void string_display(const char *title, const char *string);
+void string_display(const char *title, const char *string, unsigned short cols);
 double humanize_size(off_t bytes, const char target_unit, int precision,
 		const char **label);
-int table_display(const char *title, const alpm_list_t *header, const alpm_list_t *rows);
-void list_display(const char *title, const alpm_list_t *list);
-void list_display_linebreak(const char *title, const alpm_list_t *list);
-void signature_display(const char *title, alpm_siglist_t *siglist);
+int table_display(const char *title, const alpm_list_t *header,
+		const alpm_list_t *rows, unsigned short cols);
+void list_display(const char *title, const alpm_list_t *list,
+		unsigned short maxcols);
+void list_display_linebreak(const char *title, const alpm_list_t *list,
+		unsigned short maxcols);
+void signature_display(const char *title, alpm_siglist_t *siglist,
+		unsigned short maxcols);
 void display_targets(void);
 int str_cmp(const void *s1, const void *s2);
 void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg);
-- 
1.7.9.4



More information about the pacman-dev mailing list