commit 65cf1e801564154e05771cdf87c20fa50cf7a7af from: Stefan Sperling date: Fri Mar 16 21:33:22 2018 UTC revert the pack file handle cache again; needs more thought 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++) {