@matheusbsbs / matheus-bagliano / commits / a5f9f24aa0

pack-bitmap.c: avoid redundant calls to try_partial_reuse

try_partial_reuse() is used to mark any bits in the beginning of a bitmap whose objects can be reused verbatim from the pack they came from. Currently this function returns void, and signals nothing to the caller when bits could not be reused. But multi-pack bitmaps would benefit from having such a signal, because they may try to pass objects which are in bounds, but from a pack other than the preferred one. Any extra calls are noops because of a conditional in reuse_partial_packfile_from_bitmap(), but those loop iterations can be avoided by letting try_partial_reuse() indicate when it can't accept any more bits for reuse, and then listening to that signal. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Aug 31, 2021 at 16:52 UTC a5f9f24aa0588621770df7f45a48615e238d8e17

1 file changed +29 -11

pack-bitmap.c

+29 -11

@@ -1140,22 +1140,26 @@ cleanup:

1140 return NULL;

1141 }

1142

1143 -static void try_partial_reuse(struct bitmap_index *bitmap_git,

1144 - size_t pos,

1145 - struct bitmap *reuse,

1146 - struct pack_window **w_curs)

1143 +/*

1144 + * -1 means "stop trying further objects"; 0 means we may or may not have

1145 + * reused, but you can keep feeding bits.

1146 + */

1147 +static int try_partial_reuse(struct bitmap_index *bitmap_git,

1148 + size_t pos,

1149 + struct bitmap *reuse,

1150 + struct pack_window **w_curs)

1151 {

1152 off_t offset, header;

1153 enum object_type type;

1154 unsigned long size;

1155

1156 if (pos >= bitmap_num_objects(bitmap_git))

1153 - return; /* not actually in the pack or MIDX */

1157 + return -1; /* not actually in the pack or MIDX */

1158

1159 offset = header = pack_pos_to_offset(bitmap_git->pack, pos);

1160 type = unpack_object_header(bitmap_git->pack, w_curs, &offset, &size);

1161 if (type < 0)

1158 - return; /* broken packfile, punt */

1162 + return -1; /* broken packfile, punt */

1163

1164 if (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA) {

1165 off_t base_offset;

@@ -1172,9 +1176,9 @@ static void try_partial_reuse(struct bitmap_index *bitmap_git,

1176 base_offset = get_delta_base(bitmap_git->pack, w_curs,

1177 &offset, type, header);

1178 if (!base_offset)

1175 - return;

1179 + return 0;

1180 if (offset_to_pack_pos(bitmap_git->pack, base_offset, &base_pos) < 0)

1177 - return;

1181 + return 0;

1182

1183 /*

1184 * We assume delta dependencies always point backwards. This

@@ -1186,7 +1190,7 @@ static void try_partial_reuse(struct bitmap_index *bitmap_git,

1190 * odd parameters.

1191 */

1192 if (base_pos >= pos)

1189 - return;

1193 + return 0;

1194

1195 /*

1196 * And finally, if we're not sending the base as part of our

@@ -1197,13 +1201,14 @@ static void try_partial_reuse(struct bitmap_index *bitmap_git,

1201 * object_entry code path handle it.

1202 */

1203 if (!bitmap_get(reuse, base_pos))

1200 - return;

1204 + return 0;

1205 }

1206

1207 /*

1208 * If we got here, then the object is OK to reuse. Mark it.

1209 */

1210 bitmap_set(reuse, pos);

1211 + return 0;

1212 }

1213

1214 int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,

@@ -1239,10 +1244,23 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,

1244 break;

1245

1246 offset += ewah_bit_ctz64(word >> offset);

1242 - try_partial_reuse(bitmap_git, pos + offset, reuse, &w_curs);

1247 + if (try_partial_reuse(bitmap_git, pos + offset, reuse,

1248 + &w_curs) < 0) {

1249 + /*

1250 + * try_partial_reuse indicated we couldn't reuse

1251 + * any bits, so there is no point in trying more

1252 + * bits in the current word, or any other words

1253 + * in result.

1254 + *

1255 + * Jump out of both loops to avoid future

1256 + * unnecessary calls to try_partial_reuse.

1257 + */

1258 + goto done;

1259 + }

1260 }

1261 }

1262

1263 +done:

1264 unuse_pack(&w_curs);

1265

1266 *entries = bitmap_popcount(reuse);