Re: [PATCH 06/10] pack-objects: learn about pack index version 2
From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:43:03
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH 06/10] pack-objects: learn about pack index version 2 · Nicolas Pitre <hidden>
On Sun, 8 Apr 2007, Junio C Hamano wrote:
Nicolas Pitre [off-list ref] writes:quoted
Support for large packs exceeding 31 bits in size won't impose an index size bloat for packs within that range that don't need a 64-bit offset. And because newer objects which are likely to be the most frequently used are located at the beginning of the pack, they won't pay the 64-bit offset lookup at run time either even if the pack is large. Right now an index version 2 is created only when the biggest offset in a pack reaches 31 bits. It might be a good idea to always use index version 2 eventually to benefit from the CRC32 it contains when reusing pack data while repacking. ...@@ -582,6 +602,18 @@ static void write_index_file(void) struct object_entry **list = sorted_by_sha; struct object_entry **last = list + nr_result; uint32_t array[256]; + uint32_t index_version; + + /* if last object's offset is >= 2^31 we should use index V2 */ + index_version = (objects[nr_result-1].offset >> 31) ? 2 : 1;Although write_pack_file() iterates objects[] array in the ascending order of index and calls write_one() for each of them, in the presense of "we write base object before delta" rule, is it always true that the last object in the objects[] array has the largest offset?
Oops, indeed it is not. It is true in the index-pack but not here. Please could you amend this patch with the following:
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index c7e0a42..8cf2871 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c@@ -548,11 +548,11 @@ static off_t write_one(struct sha1file *f, return offset + size; } -static void write_pack_file(void) +static off_t write_pack_file(void) { uint32_t i; struct sha1file *f; - off_t offset; + off_t offset, last_obj_offset = 0; struct pack_header hdr; unsigned last_percent = 999; int do_progress = progress;
@@ -575,6 +575,7 @@ static void write_pack_file(void) if (!nr_result) goto done; for (i = 0; i < nr_objects; i++) { + last_obj_offset = offset; offset = write_one(f, objects + i, offset); if (do_progress) { unsigned percent = written * 100 / nr_result;
@@ -592,9 +593,11 @@ static void write_pack_file(void) if (written != nr_result) die("wrote %u objects while expecting %u", written, nr_result); sha1close(f, pack_file_sha1, 1); + + return last_obj_offset; } -static void write_index_file(void) +static void write_index_file(off_t last_obj_offset) { uint32_t i; struct sha1file *f = sha1create("%s-%s.%s", base_name,
@@ -605,7 +608,7 @@ static void write_index_file(void) uint32_t index_version; /* if last object's offset is >= 2^31 we should use index V2 */ - index_version = (objects[nr_result-1].offset >> 31) ? 2 : 1; + index_version = (last_obj_offset >> 31) ? 2 : 1; /* index versions 2 and above need a header */ if (index_version >= 2) {
@@ -1769,6 +1772,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) if (reuse_cached_pack(object_list_sha1)) ; else { + off_t last_obj_offset; if (nr_result) prepare_pack(window, depth); if (progress == 1 && pack_to_stdout) {
@@ -1778,9 +1782,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) signal(SIGALRM, SIG_IGN ); progress_update = 0; } - write_pack_file(); + last_obj_offset = write_pack_file(); if (!pack_to_stdout) { - write_index_file(); + write_index_file(last_obj_offset); puts(sha1_to_hex(object_list_sha1)); } }
Nicolas