Re: Question: .idx without .pack causes performance issues?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> I however do not think that we mark the in-core structure that
> corresponds to an open ".idx" file in any way when such a failure
> happens.  If we really cared enough, we could do so, saying "we know
> there is .idx file, but do not bother looking at it again, as we
> know the corresponding .pack is missing", and that would speed things
> up a bit, essentially bringing us back to a sane situation without
> any ".idx" without corresponding ".pack".
>
> I do not think it is worth the effort, though.  It would be more
> fruitful to find out how you end up with ".idx exists but not
> corresponding .pack" and if that is some systemic failure, see if
> there is a way to prevent that from happening in the first place.

While I still think that it is more important to prevent such a
situation from occurring in the first place, ignoring .idx that lack
corresponding .pack should be fairly simple, perhaps like this.

Note that if we wanted to do this for real, I think such an ".idx"
file should also be added to the "garbage" list in the loop in which
the second hunk of the following patch appears.

 sha1_file.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sha1_file.c b/sha1_file.c
index 1cee438..b69298e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1240,6 +1240,19 @@ static void report_pack_garbage(struct string_list *list)
 	report_helper(list, seen_bits, first, list->nr);
 }
 
+static int packfile_exists(const char *base, size_t base_len)
+{
+	struct strbuf path = STRBUF_INIT;
+	int status;
+
+	strbuf_add(&path, base, base_len);
+	strbuf_addstr(&path, ".pack");
+	status = file_exists(path.buf);
+
+	strbuf_release(&path);
+	return status;
+}
+
 static void prepare_packed_git_one(char *objdir, int local)
 {
 	struct strbuf path = STRBUF_INIT;
@@ -1281,6 +1294,7 @@ static void prepare_packed_git_one(char *objdir, int local)
 					break;
 			}
 			if (p == NULL &&
+			    packfile_exists(path.buf, base_len) &&
 			    /*
 			     * See if it really is a valid .idx file with
 			     * corresponding .pack file that we can map.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]