@matheusbsbs / matheus-bagliano / commits / ec9d224903

fsck: use streaming interface for large blobs in pack

For blobs, we want to make sure the on-disk data is not corrupted (i.e. can be inflated and produce the expected SHA-1). Blob content is opaque, there's nothing else inside to check for. For really large blobs, we may want to avoid unpacking the entire blob in memory, just to check whether it produces the same SHA-1. On 32-bit systems, we may not have enough virtual address space for such memory allocation. And even on 64-bit where it's not a problem, allocating a lot more memory could result in kicking other parts of systems to swap file, generating lots of I/O and slowing everything down. For this particular operation, not unpacking the blob and letting check_sha1_signature, which supports streaming interface, do the job is sufficient. check_sha1_signature() is not shown in the diff, unfortunately. But if will be called when "data_valid && !data" is false. We will call the callback function "fn" with NULL as "data". The only callback of this function is fsck_obj_buffer(), which does not touch "data" at all if it's a blob. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jul 13, 2016 at 17:44 UTC ec9d224903053e045d99c36149703501098b021c

4 files changed +29 -6

builtin/fsck.c

@@ -356,6 +356,10 @@ static int fsck_sha1(const unsigned char *sha1)

356 static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,

357 unsigned long size, void *buffer, int *eaten)

358 {

359 + /*

360 + * Note, buffer may be NULL if type is OBJ_BLOB. See

361 + * verify_packfile(), data_valid variable for details.

362 + */

363 struct object *obj;

364 obj = parse_object_buffer(sha1, type, size, buffer, eaten);

365 if (!obj) {

pack-check.c

+21 -2

@@ -105,6 +105,8 @@ static int verify_packfile(struct packed_git *p,

105 void *data;

106 enum object_type type;

107 unsigned long size;

108 + off_t curpos;

109 + int data_valid;

110

111 if (p->index_version > 1) {

112 off_t offset = entries[i].offset;

@@ -116,8 +118,25 @@ static int verify_packfile(struct packed_git *p,

118 sha1_to_hex(entries[i].sha1),

119 p->pack_name, (uintmax_t)offset);

120 }

119 - data = unpack_entry(p, entries[i].offset, &type, &size);

120 - if (!data)

121 +

122 + curpos = entries[i].offset;

123 + type = unpack_object_header(p, w_curs, &curpos, &size);

124 + unuse_pack(w_curs);

125 +

126 + if (type == OBJ_BLOB && big_file_threshold <= size) {

127 + /*

128 + * Let check_sha1_signature() check it with

129 + * the streaming interface; no point slurping

130 + * the data in-core only to discard.

131 + */

132 + data = NULL;

133 + data_valid = 0;

134 + } else {

135 + data = unpack_entry(p, entries[i].offset, &type, &size);

136 + data_valid = 1;

137 + }

138 +

139 + if (data_valid && !data)

140 err = error("cannot unpack %s from %s at offset %"PRIuMAX"",

141 sha1_to_hex(entries[i].sha1), p->pack_name,

142 (uintmax_t)entries[i].offset);

pack.h

@@ -74,6 +74,7 @@ struct pack_idx_entry {

76 struct progress;

77 +/* Note, the data argument could be NULL if object type is blob */

78 typedef int (*verify_fn)(const unsigned char*, enum object_type, unsigned long, void*, int*);

80 extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const unsigned char *sha1);

t/t1050-large.sh

+3 -4

@@ -177,10 +177,9 @@ test_expect_success 'zip achiving, deflate' '

177 git archive --format=zip HEAD >/dev/null

178 '

179

180 -test_expect_success 'fsck' '

181 - test_must_fail git fsck 2>err &&

182 - n=$(grep "error: attempting to allocate .* over limit" err | wc -l) &&

183 - test "$n" -gt 1

180 +test_expect_success 'fsck large blobs' '

181 + git fsck 2>err &&

182 + test_must_be_empty err

183 '

184

185 test_done