pack-redundant: consistent sort method
SZEDER reported that test case t5323 has different test result on MacOS. This is because `cmp_pack_list_reverse` cannot give identical result when two pack being sorted has the same size of remaining_objects. Changes to the sorting function will make consistent test result for t5323. The new algorithm to find redundant packs is a trade-off to save memory resources, and the result of it may be different with old one, and may be not the best result sometimes. Update t5323 for the new algorithm. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiang Xin committed
Feb 2, 2019 at 21:30 UTC
0e37abd2e89d475500e93a6368f1f054ca2fec55
2 files changed
+25
-17
builtin/pack-redundant.c
+16
-8
@@ -33,6 +33,7 @@ static struct pack_list {
33
struct packed_git *pack;
34
struct llist *unique_objects;
35
struct llist *remaining_objects;
36
+ size_t all_objects_size;
37
} *local_packs = NULL, *altodb_packs = NULL;
38
39
static struct llist_item *free_nodes;
@@ -340,19 +341,25 @@ static inline off_t pack_set_bytecount(struct pack_list *pl)
341
return ret;
342
}
343
343
-static int cmp_pack_list_reverse(const void *a, const void *b)
344
+static int cmp_remaining_objects(const void *a, const void *b)
345
{
346
struct pack_list *pl_a = *((struct pack_list **)a);
347
struct pack_list *pl_b = *((struct pack_list **)b);
347
- size_t sz_a = pl_a->remaining_objects->size;
348
- size_t sz_b = pl_b->remaining_objects->size;
348
350
- if (sz_a == sz_b)
351
- return 0;
352
- else if (sz_a < sz_b)
349
+ if (pl_a->remaining_objects->size == pl_b->remaining_objects->size) {
350
+ /* have the same remaining_objects, big pack first */
351
+ if (pl_a->all_objects_size == pl_b->all_objects_size)
352
+ return 0;
353
+ else if (pl_a->all_objects_size < pl_b->all_objects_size)
354
+ return 1;
355
+ else
356
+ return -1;
357
+ } else if (pl_a->remaining_objects->size < pl_b->remaining_objects->size) {
358
+ /* sort by remaining objects, more objects first */
359
return 1;
354
- else
360
+ } else {
361
return -1;
362
+ }
363
}
364
365
/* Sort pack_list, greater size of remaining_objects first */
@@ -370,7 +377,7 @@ static void sort_pack_list(struct pack_list **pl)
377
for (n = 0, p = *pl; p; p = p->next)
378
ary[n++] = p;
379
373
- QSORT(ary, n, cmp_pack_list_reverse);
380
+ QSORT(ary, n, cmp_remaining_objects);
381
382
/* link them back again */
383
for (i = 0; i < n - 1; i++)
@@ -511,6 +518,7 @@ static struct pack_list * add_pack(struct packed_git *p)
518
llist_insert_back(l.remaining_objects, (const struct object_id *)(base + off));
519
off += step;
520
}
521
+ l.all_objects_size = l.remaining_objects->size;
522
l.unique_objects = NULL;
523
if (p->pack_local)
524
return pack_list_insert(&local_packs, &l);
t/t5323-pack-redundant.sh
+9
-9
@@ -165,15 +165,15 @@ test_expect_success 'master: no redundant for pack 1, 2, 3' '
165
# | T A B C D E F G H I J K L M N O P Q R
166
# ----+--------------------------------------
167
# P1 | x x x x x x x x
168
-# P2* | ! ! ! ! ! ! !
169
-# P3 | x x x x x x
168
+# P2 | x x x x x x x
169
+# P3* | ! ! ! ! ! !
170
# P4 | x x x x x
171
# P5 | x x x x
172
# ----+--------------------------------------
173
# ALL | x x x x x x x x x x x x x x x x x x
174
#
175
#############################################################################
176
-test_expect_failure 'master: one of pack-2/pack-3 is redundant (failed on Mac)' '
176
+test_expect_success 'master: one of pack-2/pack-3 is redundant' '
177
create_pack_in "$master_repo" P4 <<-EOF &&
178
$J
179
$K
@@ -190,7 +190,7 @@ test_expect_failure 'master: one of pack-2/pack-3 is redundant (failed on Mac)'
190
(
191
cd "$master_repo" &&
192
cat >expect <<-EOF &&
193
- P2:$P2
193
+ P3:$P3
194
EOF
195
git pack-redundant --all >out &&
196
format_packfiles <out >actual &&
@@ -214,7 +214,7 @@ test_expect_failure 'master: one of pack-2/pack-3 is redundant (failed on Mac)'
214
# ALL | x x x x x x x x x x x x x x x x x x x
215
#
216
#############################################################################
217
-test_expect_failure 'master: pack 2, 4, and 6 are redundant (failed on Mac)' '
217
+test_expect_success 'master: pack 2, 4, and 6 are redundant' '
218
create_pack_in "$master_repo" P6 <<-EOF &&
219
$N
220
$O
@@ -254,7 +254,7 @@ test_expect_failure 'master: pack 2, 4, and 6 are redundant (failed on Mac)' '
254
# ALL | x x x x x x x x x x x x x x x x x x x
255
#
256
#############################################################################
257
-test_expect_failure 'master: pack-8 (subset of pack-1) is also redundant (failed on Mac)' '
257
+test_expect_success 'master: pack-8 (subset of pack-1) is also redundant' '
258
create_pack_in "$master_repo" P8 <<-EOF &&
259
$A
260
EOF
@@ -281,7 +281,7 @@ test_expect_success 'master: clean loose objects' '
281
)
282
'
283
284
-test_expect_failure 'master: remove redundant packs and pass fsck (failed on Mac)' '
284
+test_expect_success 'master: remove redundant packs and pass fsck' '
285
(
286
cd "$master_repo" &&
287
git pack-redundant --all | xargs rm &&
@@ -301,7 +301,7 @@ test_expect_success 'setup shared.git' '
301
)
302
'
303
304
-test_expect_failure 'shared: all packs are redundant, but no output without --alt-odb (failed on Mac)' '
304
+test_expect_success 'shared: all packs are redundant, but no output without --alt-odb' '
305
(
306
cd "$shared_repo" &&
307
git pack-redundant --all >out &&
@@ -334,7 +334,7 @@ test_expect_failure 'shared: all packs are redundant, but no output without --al
334
# ALL | x x x x x x x x x x x x x x x x x x x
335
#
336
#############################################################################
337
-test_expect_failure 'shared: show redundant packs in stderr for verbose mode (failed on Mac)' '
337
+test_expect_success 'shared: show redundant packs in stderr for verbose mode' '
338
(
339
cd "$shared_repo" &&
340
cat >expect <<-EOF &&