pathological direct mode behavior for files with many often changing include files

Yiding Jia <[email protected]>
Newsgroups gmane.comp.compilers.ccache
Message-ID <CF71A9DE.CCD0%[email protected]>
When direct mode is enabled and a file has many dependencies that change,
it will cause `file_info` entries to accumulate over an extended period of
time. Since the manifest file reaping is only based on the actual source
file (not its dependent files) changing, this list can grow very large.

This has two effects:

1. Ccache execution becomes slower when writing. In a simple pathological
test ccache took from 50ms to 150ms on cache misses, this includes the
underlying compile time, so actual ccache attributed performance hit is
worse. The effect is minimized during hits.
2. Ccache infinite loops while writing out manifest after 65k files. In
`write_manifest`, the counter is 16bit while n_file_infos is 32bit.

This may be a somewhat rare occurrence in the field. We use ccache for
continuous integration, and this problem only shows up for a few files
(those with many recursively included frequently changing headers) after
tens of thousands of builds. Once it starts happening, however, subsequent
builds fail as the cache is effectively poisoned.



Repro steps:

1. Create 10000 header files with some random valid contents (such as
`extern int foo_RANDOMNUMBER;`)
2. Create a c file that references those header files, and does something
trivial.
3. Compile with ccache in direct mode.
4. Goto 1.


Simple patch attached, which converts all the counters in `manifest.c` to
`uint32_t` from `uint16_t` and reaps a manifest file when the number of
file_info entries becomes larger than some threshold. I have not
experimented with this threshold, but 10000 should accommodate a sizable
number of file changes.

_______________________________________________
ccache mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/ccache
0001-manifest-file_info-fix.patch (application/octet-stream, 1.9 KB)
From ae9c2a98d58b619e883c62a84ebae86ab53c8da7 Mon Sep 17 00:00:00 2001
From: Yiding Jia <[email protected]>
Date: Mon, 14 Apr 2014 14:46:46 -0700
Subject: [PATCH] manifest file_info fix

---
 manifest.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/manifest.c b/manifest.c
index 1327170..0e4f53d 100644
--- a/manifest.c
+++ b/manifest.c
@@ -68,6 +68,7 @@
 
 static const uint32_t MAGIC = 0x63436d46U;
 static const uint32_t MAX_MANIFEST_ENTRIES = 100;
+static const uint32_t MAX_MANIFEST_FILE_INFO_ENTRIES = 10000;
 
 #define ccache_static_assert(e) \
 	do { enum { ccache_static_assert__ = 1/(e) }; } while (false)
@@ -145,7 +146,7 @@ file_infos_equal(void *key1, void *key2)
 static void
 free_manifest(struct manifest *mf)
 {
-	uint16_t i;
+	uint32_t i;
 	for (i = 0; i < mf->n_files; i++) {
 		free(mf->files[i]);
 	}
@@ -238,7 +239,7 @@ static struct manifest *
 read_manifest(gzFile f)
 {
 	struct manifest *mf;
-	uint16_t i, j;
+	uint32_t i, j;
 	uint32_t magic;
 
 	mf = create_empty_manifest();
@@ -336,7 +337,7 @@ error:
 static int
 write_manifest(gzFile f, const struct manifest *mf)
 {
-	uint16_t i, j;
+	uint32_t i, j;
 
 	WRITE_INT(4, MAGIC);
 	WRITE_INT(1, MANIFEST_VERSION);
@@ -724,6 +725,15 @@ manifest_put(const char *manifest_path, struct file_hash *object_hash,
 		       MAX_MANIFEST_ENTRIES);
 		free_manifest(mf);
 		mf = create_empty_manifest();
+	} else if (mf->n_file_infos > MAX_MANIFEST_FILE_INFO_ENTRIES) {
+		/* Rarely, file_info entries can grow large in pathological cases where
+		 * many included files change, but the main file does not. This also puts
+		 * an upper bound on th enumber of file_info entries.
+		 */
+		cc_log("More than %u file_info entries in manifest file; discarding",
+		       MAX_MANIFEST_FILE_INFO_ENTRIES);
+		free_manifest(mf);
+		mf = create_empty_manifest();
 	}
 
 	tmp_file = format("%s.tmp.%s", manifest_path, tmp_string());
-- 
1.8.5.2 (Apple Git-48)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.