op public repos

Commit Diff

Commit:: 65cf1e801564154e05771cdf87c20fa50cf7a7af
From:: Stefan Sperling <stsp@stsp.name>
Date:: Fri Mar 16 21:33:22 2018 UTC
Message:: revert the pack file handle cache again; needs more thought
Actions:: Patch | Tree
commit - ef2bccd93f110957437a89413bd4e27853c396b6
commit + 65cf1e801564154e05771cdf87c20fa50cf7a7af
blob - 18440f457888c07df41f5d7aa535c94624dce102
blob + a8e4cedf77becb8285b6605ada60fbe549208fbb
--- lib/got_repository_lib.h
+++ lib/got_repository_lib.h
@@ -27,25 +27,16 @@ struct got_delta_cache {
 	struct got_delta_cache_entry deltas[GOT_DELTA_CACHE_SIZE];
 };
 
-#define GOT_PACK_CACHE_SIZE	64
+#define GOT_PACKIDX_CACHE_SIZE	64
 
-struct got_pack_cache {
-	struct got_packidx_v2_hdr *packidx;
-	char *path_packfile;
-	FILE *packfile;
-};
-
 struct got_repository {
 	char *path;
 	char *path_git_dir;
 
-	/* 
-	 * The pack cache speeds up search for packed objects and
-	 * avoids duplicate open file handles to pack files.
-	 */
-	struct got_pack_cache pack_cache[GOT_PACK_CACHE_SIZE];
+	/* The pack index cache speeds up search for packed objects. */
+	struct got_packidx_v2_hdr *packidx_cache[GOT_PACKIDX_CACHE_SIZE];
 
 	/* The delta cache speeds up reconstruction of packed objects. */
-	struct got_delta_cache delta_cache[GOT_PACK_CACHE_SIZE];
+	struct got_delta_cache delta_cache[GOT_PACKIDX_CACHE_SIZE];
 };
 
blob - f8b51cebf8470a34f67abea5e6fe82cd4d697f26
blob + a8cbf633e11797b009036d0963079169cbb8ebd0
--- lib/pack.c
+++ lib/pack.c
@@ -362,101 +362,27 @@ err:
 	free(p->sorted_ids);
 	free(p);
 	return NULL;
-}
-
-static const struct got_error *
-get_packfile_path(char **path_packfile, struct got_repository *repo,
-    struct got_packidx_v2_hdr *packidx)
-{
-	char *path_packdir;
-	char hex[SHA1_DIGEST_STRING_LENGTH];
-	char *sha1str;
-
-	path_packdir = got_repo_get_path_objects_pack(repo);
-	if (path_packdir == NULL)
-		return got_error(GOT_ERR_NO_MEM);
-
-	sha1str = got_sha1_digest_to_str(packidx->trailer.packfile_sha1,
-	    hex, sizeof(hex));
-	if (sha1str == NULL)
-		return got_error(GOT_ERR_PACKIDX_CSUM);
-
-	if (asprintf(path_packfile, "%s/%s%s%s", path_packdir,
-	    GOT_PACK_PREFIX, sha1str, GOT_PACKFILE_SUFFIX) == -1) {
-		*path_packfile = NULL;
-		return got_error(GOT_ERR_NO_MEM);
-	}
-
-	return NULL;
-}
-
-static const struct got_error *
-read_packfile_hdr(FILE *f, struct got_packidx_v2_hdr *packidx)
-{
-	const struct got_error *err = NULL;
-	uint32_t totobj = betoh32(packidx->fanout_table[0xff]);
-	struct got_packfile_hdr hdr;
-	size_t n;
-
-	n = fread(&hdr, sizeof(hdr), 1, f);
-	if (n != 1)
-		return got_ferror(f, GOT_ERR_BAD_PACKIDX);
-
-	if (betoh32(hdr.signature) != GOT_PACKFILE_SIGNATURE ||
-	    betoh32(hdr.version) != GOT_PACKFILE_VERSION ||
-	    betoh32(hdr.nobjects) != totobj)
-		err = got_error(GOT_ERR_BAD_PACKFILE);
-
-	return err;
 }
 
-static const struct got_error *
-cache_pack(struct got_packidx_v2_hdr *packidx, struct got_repository *repo)
+static void
+cache_packidx(struct got_packidx_v2_hdr *packidx, struct got_repository *repo)
 {
-	const struct got_error *err;
-	FILE *packfile;
-	char *path_packfile;
 	int i;
 
-	err = get_packfile_path(&path_packfile, repo, packidx);
-	if (err)
-		return err;
-
-	packfile = fopen(path_packfile, "rb");
-	if (packfile == NULL) {
-		err = got_error_from_errno();
-		free(path_packfile);
-		return err;
-	}
-	err = read_packfile_hdr(packfile, packidx);
-	if (err) {
-		fclose(packfile);
-		return err;
-	}
-
-	for (i = 0; i < nitems(repo->pack_cache); i++) {
-		if (repo->pack_cache[i].packidx == NULL)
+	for (i = 0; i < nitems(repo->packidx_cache); i++) {
+		if (repo->packidx_cache[i] == NULL)
 			break;
 	}
 
-	if (i == nitems(repo->pack_cache)) {
-		got_packidx_close(repo->pack_cache[i - 1].packidx);
-		fclose(repo->pack_cache[i - 1].packfile);
-		free(repo->pack_cache[i - 1].path_packfile);
-		memmove(&repo->pack_cache[1], &repo->pack_cache[0],
-		    sizeof(repo->pack_cache) - sizeof(repo->pack_cache[0]));
+	if (i == nitems(repo->packidx_cache)) {
+		got_packidx_close(repo->packidx_cache[i - 1]);
+		memmove(&repo->packidx_cache[1], &repo->packidx_cache[0],
+		    sizeof(repo->packidx_cache) -
+		    sizeof(repo->packidx_cache[0]));
 		i = 0;
 	}
 
-	repo->pack_cache[i].packidx = dup_packidx(packidx);
-	if (repo->pack_cache[i].packidx != NULL) {
-		repo->pack_cache[i].packfile = packfile;
-		repo->pack_cache[i].path_packfile = path_packfile;
-	} else {
-		fclose(packfile);
-		free(path_packfile);
-	}
-	return NULL;
+	repo->packidx_cache[i] = dup_packidx(packidx);
 }
 
 static const struct got_error *
@@ -470,13 +396,13 @@ search_packidx(struct got_packidx_v2_hdr **packidx, in
 	char *path_packidx;
 	int i;
 
-	/* Search pack cache. */
-	for (i = 0; i < nitems(repo->pack_cache); i++) {
-		if (repo->pack_cache[i].packidx == NULL)
+	/* Search pack index cache. */
+	for (i = 0; i < nitems(repo->packidx_cache); i++) {
+		if (repo->packidx_cache[i] == NULL)
 			break;
-		*idx = get_object_idx(repo->pack_cache[i].packidx, id);
+		*idx = get_object_idx(repo->packidx_cache[i], id);
 		if (*idx != -1) {
-			*packidx = dup_packidx(repo->pack_cache[i].packidx);
+			*packidx = dup_packidx(repo->packidx_cache[i]);
 			if (*packidx == NULL)
 				return got_error(GOT_ERR_NO_MEM);
 			return NULL;
@@ -512,7 +438,7 @@ search_packidx(struct got_packidx_v2_hdr **packidx, in
 		*idx = get_object_idx(*packidx, id);
 		if (*idx != -1) {
 			err = NULL; /* found the object */
-			cache_pack(*packidx, repo);
+			cache_packidx(*packidx, repo);
 			goto done;
 		}
 
@@ -529,34 +455,59 @@ done:
 }
 
 static const struct got_error *
-open_packfile(FILE **packfile, char **path_packfile, int *is_cached,
-    struct got_repository *repo, struct got_packidx_v2_hdr *packidx)
+get_packfile_path(char **path_packfile, struct got_repository *repo,
+    struct got_packidx_v2_hdr *packidx)
 {
-	const struct got_error *err;
-	int i;
+	char *path_packdir;
+	char hex[SHA1_DIGEST_STRING_LENGTH];
+	char *sha1str;
 
-	*packfile = NULL;
-	*path_packfile = NULL;
-	*is_cached = 0;
+	path_packdir = got_repo_get_path_objects_pack(repo);
+	if (path_packdir == NULL)
+		return got_error(GOT_ERR_NO_MEM);
 
-	/* The pack could already be cached after an object search. */
-	for (i = 0; i < nitems(repo->pack_cache); i++) {
-		if (repo->pack_cache[i].packidx == NULL)
-			break;
+	sha1str = got_sha1_digest_to_str(packidx->trailer.packfile_sha1,
+	    hex, sizeof(hex));
+	if (sha1str == NULL)
+		return got_error(GOT_ERR_PACKIDX_CSUM);
 
-		/* The pack index trailer acts as a cache key. */
-		if (memcmp(&packidx->trailer,
-		    &repo->pack_cache[i].packidx->trailer,
-		    sizeof(packidx->trailer)) != 0)
-			continue;
-
-		*packfile = repo->pack_cache[i].packfile;
-		*path_packfile = repo->pack_cache[i].path_packfile;
-		*is_cached = 1;
-		return NULL;
+	if (asprintf(path_packfile, "%s/%s%s%s", path_packdir,
+	    GOT_PACK_PREFIX, sha1str, GOT_PACKFILE_SUFFIX) == -1) {
+		*path_packfile = NULL;
+		return got_error(GOT_ERR_NO_MEM);
 	}
-	/* No luck. Try the filesystem. */
 
+	return NULL;
+}
+
+const struct got_error *
+read_packfile_hdr(FILE *f, struct got_packidx_v2_hdr *packidx)
+{
+	const struct got_error *err = NULL;
+	uint32_t totobj = betoh32(packidx->fanout_table[0xff]);
+	struct got_packfile_hdr hdr;
+	size_t n;
+
+	n = fread(&hdr, sizeof(hdr), 1, f);
+	if (n != 1)
+		return got_ferror(f, GOT_ERR_BAD_PACKIDX);
+
+	if (betoh32(hdr.signature) != GOT_PACKFILE_SIGNATURE ||
+	    betoh32(hdr.version) != GOT_PACKFILE_VERSION ||
+	    betoh32(hdr.nobjects) != totobj)
+		err = got_error(GOT_ERR_BAD_PACKFILE);
+
+	return err;
+}
+
+static const struct got_error *
+open_packfile(FILE **packfile, char **path_packfile,
+    struct got_repository *repo, struct got_packidx_v2_hdr *packidx)
+{
+	const struct got_error *err;
+
+	*packfile = NULL;
+
 	err = get_packfile_path(path_packfile, repo, packidx);
 	if (err)
 		return err;
@@ -762,9 +713,8 @@ resolve_ref_delta(struct got_delta_chain *deltas, stru
 	uint64_t base_size;
 	size_t base_tslen;
 	size_t n;
-	FILE *base_packfile = NULL;
-	char *path_base_packfile = NULL;
-	int pack_cached = 0;
+	FILE *base_packfile;
+	char *path_base_packfile;
 	off_t delta_data_offset;
 
 	n = fread(&id, sizeof(id), 1, packfile);
@@ -790,8 +740,7 @@ resolve_ref_delta(struct got_delta_chain *deltas, stru
 		return got_error(GOT_ERR_BAD_PACKIDX);
 	}
 
-	err = open_packfile(&base_packfile, &path_base_packfile, &pack_cached,
-	    repo, packidx);
+	err = open_packfile(&base_packfile, &path_base_packfile, repo, packidx);
 	got_packidx_close(packidx);
 	if (err)
 		return err;
@@ -810,11 +759,9 @@ resolve_ref_delta(struct got_delta_chain *deltas, stru
 	    path_base_packfile, base_offset, base_tslen, base_type,
 	    base_size);
 done:
-	if (!pack_cached) {
-		free(path_base_packfile);
-		if (base_packfile && fclose(base_packfile) == -1 && err == 0)
-			err = got_error_from_errno();
-	}
+	free(path_base_packfile);
+	if (base_packfile && fclose(base_packfile) == -1 && err == 0)
+		err = got_error_from_errno();
 	return err;
 }
 
@@ -904,9 +851,8 @@ open_packed_object(struct got_object **obj, struct got
 {
 	const struct got_error *err = NULL;
 	off_t offset;
-	char *path_packfile = NULL;
-	FILE *packfile = NULL;
-	int pack_cached = 0;
+	char *path_packfile;
+	FILE *packfile;
 	uint8_t type;
 	uint64_t size;
 	size_t tslen;
@@ -917,8 +863,7 @@ open_packed_object(struct got_object **obj, struct got
 	if (offset == (uint64_t)-1)
 		return got_error(GOT_ERR_BAD_PACKIDX);
 
-	err = open_packfile(&packfile, &path_packfile, &pack_cached,
-	    repo, packidx);
+	err = open_packfile(&packfile, &path_packfile, repo, packidx);
 	if (err)
 		return err;
 
@@ -951,11 +896,9 @@ open_packed_object(struct got_object **obj, struct got
 		goto done;
 	}
 done:
-	if (!pack_cached) {
-		free(path_packfile);
-		if (packfile && fclose(packfile) == -1 && err == 0)
-			err = got_error_from_errno();
-	}
+	free(path_packfile);
+	if (packfile && fclose(packfile) == -1 && err == 0)
+		err = got_error_from_errno();
 	return err;
 }
 
blob - 267fc8578534c171d919197590ca0989477d46a9
blob + c19c01392d63487dbce490ca2de4bbd2576af1d0
--- lib/repository.c
+++ lib/repository.c
@@ -202,12 +202,10 @@ got_repo_close(struct got_repository *repo)
 {
 	int i;
 
-	for (i = 0; i < nitems(repo->pack_cache); i++) {
-		if (repo->pack_cache[i].packidx == NULL)
+	for (i = 0; i < nitems(repo->packidx_cache); i++) {
+		if (repo->packidx_cache[i] == NULL)
 			break;
-		got_packidx_close(repo->pack_cache[i].packidx);
-		fclose(repo->pack_cache[i].packfile);
-		free(repo->pack_cache[i].path_packfile);
+		got_packidx_close(repo->packidx_cache[i]);
 	}
 
 	for (i = 0; i < nitems(repo->delta_cache); i++) {