From: Rusty Russell <rusty@rustcorp.com.au>
Date: Mon, 10 Sep 2012 05:03:01 +0000 (+0930)
Subject: tools: more intelligent caching for compile _info.
X-Git-Url: https://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=e2a53df3e1f5912c872f1906912053354d0d0c63

tools: more intelligent caching for compile _info.

We let the get_deps() caller hand us the compiled _info filename, but what
about recursive dependencies?  These we re-generate every time.

So fix this: hand a generator callback to get_deps(), expose the one
which simply compiles it, and add a ccanlint one which looks up the
manifest to see if we have one already.

Before:
$ ccanlint -vvvv ccan/failtest | grep -c 'Creating.*_info'
31

After:
$ ccanlint -vvvv ccan/failtest | grep -c 'Creating.*_info'
17
---

diff --git a/tools/ccan_depends.c b/tools/ccan_depends.c
index 4dda6fa2..c0b0f94b 100644
--- a/tools/ccan_depends.c
+++ b/tools/ccan_depends.c
@@ -38,7 +38,7 @@ int main(int argc, char *argv[])
 
 	if (compile)
 		deps = get_deps(talloc_autofree_context(), argv[1],
-				recurse, NULL);
+				recurse, compile_info);
 	else
 		deps = get_safe_ccan_deps(talloc_autofree_context(),
 					  argv[1], recurse);
diff --git a/tools/ccanlint/ccanlint.h b/tools/ccanlint/ccanlint.h
index 8ebf68d2..954bb30a 100644
--- a/tools/ccanlint/ccanlint.h
+++ b/tools/ccanlint/ccanlint.h
@@ -164,6 +164,9 @@ void compile_and_link_async(const void *ctx, unsigned int time_ms,
 /* Get results of a command, returning ctx (and free it). */
 void *collect_command(bool *ok, char **output);
 
+/* Find manifest for this dir and return compiled _info filename. */
+char *get_or_compile_info(const void *ctx, const char *dir);
+
 /* Normal tests. */
 extern struct ccanlint trailing_whitespace;
 
diff --git a/tools/ccanlint/file_analysis.c b/tools/ccanlint/file_analysis.c
index 8760630a..575425df 100644
--- a/tools/ccanlint/file_analysis.c
+++ b/tools/ccanlint/file_analysis.c
@@ -394,3 +394,13 @@ void score_file_error(struct score *score, struct ccan_file *f, unsigned line,
 		score->error = talloc_append_string(score->error,
 				    "... more (use -vv to see them all)\n");
 }
+
+char *get_or_compile_info(const void *ctx, const char *dir)
+{
+	struct manifest *m = get_manifest(NULL, dir);
+
+	if (!m->info_file->compiled[COMPILE_NORMAL])
+		m->info_file->compiled[COMPILE_NORMAL] = compile_info(m, dir);
+
+	return m->info_file->compiled[COMPILE_NORMAL];
+}
diff --git a/tools/ccanlint/tests/depends_exist.c b/tools/ccanlint/tests/depends_exist.c
index 27bbfc4e..7712c48d 100644
--- a/tools/ccanlint/tests/depends_exist.c
+++ b/tools/ccanlint/tests/depends_exist.c
@@ -57,8 +57,7 @@ static void check_depends_exist(struct manifest *m,
 	if (safe_mode)
 		deps = get_safe_ccan_deps(m, m->dir, true);
 	else
-		deps = get_deps(m, m->dir, true,
-				&m->info_file->compiled[COMPILE_NORMAL]);
+		deps = get_deps(m, m->dir, true, get_or_compile_info);
 
 	for (i = 0; deps[i]; i++) {
 		if (!strstarts(deps[i], "ccan/"))
diff --git a/tools/ccanlint/tests/examples_compile.c b/tools/ccanlint/tests/examples_compile.c
index 6602c546..7fb60845 100644
--- a/tools/ccanlint/tests/examples_compile.c
+++ b/tools/ccanlint/tests/examples_compile.c
@@ -62,8 +62,7 @@ static void add_dep(struct manifest ***deps, const char *basename)
 	if (m->info_file) {
 		char **infodeps;
 
-		infodeps = get_deps(m, m->dir, false,
-				    &m->info_file->compiled[COMPILE_NORMAL]);
+		infodeps = get_deps(m, m->dir, false, get_or_compile_info);
 
 		for (i = 0; infodeps[i]; i++) {
 			if (strstarts(infodeps[i], "ccan/"))
diff --git a/tools/depends.c b/tools/depends.c
index 7f232233..6958c37b 100644
--- a/tools/depends.c
+++ b/tools/depends.c
@@ -38,7 +38,7 @@ lines_from_cmd(const void *ctx, const char *format, ...)
 
 /* Be careful about trying to compile over running programs (parallel make).
  * temp_file helps here. */
-static char *compile_info(const void *ctx, const char *dir)
+char *compile_info(const void *ctx, const char *dir)
 {
 	char *info_c_file, *info, *ccandir, *compiled, *output;
 	size_t len;
@@ -71,17 +71,16 @@ static char *compile_info(const void *ctx, const char *dir)
 	return NULL;
 }
 
-static char **get_one_deps(const void *ctx, const char *dir, char **infofile)
+static char **get_one_deps(const void *ctx, const char *dir,
+			   char *(*get_info)(const void *ctx, const char *dir))
 {
 	char **deps, *cmd;
+	char *infofile = get_info(ctx, dir);
 
-	if (!*infofile) {
-		*infofile = compile_info(ctx, dir);
-		if (!*infofile)
-			errx(1, "Could not compile _info for '%s'", dir);
-	}
+	if (!infofile)
+		return NULL;
 
-	cmd = talloc_asprintf(ctx, "%s depends", *infofile);
+	cmd = talloc_asprintf(ctx, "%s depends", infofile);
 	deps = lines_from_cmd(cmd, "%s", cmd);
 	if (!deps)
 		err(1, "Could not run '%s'", cmd);
@@ -118,7 +117,7 @@ static char *replace(const void *ctx, const char *src,
 /* This is a terrible hack.  We scan for ccan/ strings. */
 static char **get_one_safe_deps(const void *ctx,
 				const char *dir,
-				char **infofile)
+				char *(*unused)(const void *, const char *))
 {
 	char **deps, **lines, *raw, *fname;
 	unsigned int i, n;
@@ -178,17 +177,17 @@ static bool have_dep(char **deps, const char *dep)
 /* Gets all the dependencies, recursively. */
 static char **
 get_all_deps(const void *ctx, const char *dir,
-	     char **infofile,
-	     char **(*get_one)(const void *, const char *, char **))
+	     char *(*get_info)(const void *ctx, const char *dir),
+	     char **(*get_one)(const void *, const char *,
+			       char *(*get_info)(const void *, const char *)))
 {
 	char **deps;
 	unsigned int i;
 
-	deps = get_one(ctx, dir, infofile);
+	deps = get_one(ctx, dir, get_info);
 	for (i = 0; i < talloc_array_length(deps)-1; i++) {
 		char **newdeps;
 		unsigned int j;
-		char *subinfo = NULL;
 		char *subdir;
 
 		if (!strstarts(deps[i], "ccan/"))
@@ -197,7 +196,7 @@ get_all_deps(const void *ctx, const char *dir,
 		subdir = talloc_asprintf(ctx, "%s/%s",
 					 talloc_dirname(ctx, dir),
 					 deps[i] + strlen("ccan/"));
-		newdeps = get_one(ctx, subdir, &subinfo);
+		newdeps = get_one(ctx, subdir, get_info);
 
 		/* Should be short, so brute-force out dups. */
 		for (j = 0; j < talloc_array_length(newdeps)-1; j++) {
@@ -240,6 +239,9 @@ static char **uniquify_deps(char **deps)
 {
 	unsigned int i, j, num;
 
+	if (!deps)
+		return NULL;
+
 	num = talloc_array_length(deps) - 1;
 	for (i = 0; i < num; i++) {
 		for (j = i + 1; j < num; j++) {
@@ -255,22 +257,16 @@ static char **uniquify_deps(char **deps)
 	return talloc_realloc(NULL, deps, char *, num + 1);
 }
 
-char **get_deps(const void *ctx, const char *dir,
-		bool recurse, char **infofile)
+char **get_deps(const void *ctx, const char *dir, bool recurse,
+		char *(*get_info)(const void *ctx, const char *dir))
 {
-	char *temp = NULL, **ret;
-	if (!infofile)
-		infofile = &temp;
+	char **ret;
 
 	if (!recurse) {
-		ret = get_one_deps(ctx, dir, infofile);
+		ret = get_one_deps(ctx, dir, get_info);
 	} else
-		ret = get_all_deps(ctx, dir, infofile, get_one_deps);
+		ret = get_all_deps(ctx, dir, get_info, get_one_deps);
 
-	if (infofile == &temp && temp) {
-		unlink(temp);
-		talloc_free(temp);
-	}
 	return uniquify_deps(ret);
 }
 
diff --git a/tools/namespacize.c b/tools/namespacize.c
index 100d7942..931d05f3 100644
--- a/tools/namespacize.c
+++ b/tools/namespacize.c
@@ -458,7 +458,7 @@ static void adjust_dir(const char *dir)
 
 	verbose("Adjusting %s\n", dir);
 	verbose_indent();
-	for (deps = get_deps(parent, dir, false, NULL); *deps; deps++) {
+	for (deps = get_deps(parent, dir, false, compile_info); *deps; deps++) {
 		char *depdir;
 		struct adjusted *adj = NULL;
 		struct replace *repl;
@@ -496,7 +496,7 @@ static void adjust_dependents(const char *dir)
 		if (access(info, R_OK) != 0)
 			continue;
 
-		for (deps = get_deps(*file, *file, false, NULL);
+		for (deps = get_deps(*file, *file, false, compile_info);
 		     *deps; deps++) {
 			if (!strstarts(*deps, "ccan/"))
 				continue;
diff --git a/tools/tools.h b/tools/tools.h
index 8d1ae048..e78d1390 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -19,9 +19,12 @@
 
 #define COVERAGE_CFLAGS "-fprofile-arcs -ftest-coverage"
 
+/* This compiles up the _info file into a temporary. */
+char *compile_info(const void *ctx, const char *dir);
+
 /* This actually compiles and runs the info file to get dependencies. */
 char **get_deps(const void *ctx, const char *dir, bool recurse,
-		char **infofile);
+		char *(*get_info)(const void *ctx, const char *dir));
 
 /* This is safer: just looks for ccan/ strings in info */
 char **get_safe_ccan_deps(const void *ctx, const char *dir, bool recurse);