Skip to content

Commit 0df82d9

Browse files
committed
Merge branch 'jk/object-filter-with-bitmap'
The object reachability bitmap machinery and the partial cloning machinery were not prepared to work well together, because some object-filtering criteria that partial clones use inherently rely on object traversal, but the bitmap machinery is an optimization to bypass that object traversal. There however are some cases where they can work together, and they were taught about them. * jk/object-filter-with-bitmap: rev-list --count: comment on the use of count_right++ pack-objects: support filters with bitmaps pack-bitmap: implement BLOB_LIMIT filtering pack-bitmap: implement BLOB_NONE filtering bitmap: add bitmap_unset() function rev-list: use bitmap filters for traversal pack-bitmap: basic noop bitmap filter infrastructure rev-list: allow commit-only bitmap traversals t5310: factor out bitmap traversal comparison rev-list: allow bitmaps when counting objects rev-list: make --count work with --objects rev-list: factor out bitmap-optimized routines pack-bitmap: refuse to do a bitmap traversal with pathspecs rev-list: fallback to non-bitmap traversal when filtering pack-bitmap: fix leak of haves/wants object lists pack-bitmap: factor out type iterator initialization
2 parents 80648bb + 20a5fd8 commit 0df82d9

14 files changed

+511
-70
lines changed

builtin/pack-objects.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3184,7 +3184,7 @@ static int pack_options_allow_reuse(void)
31843184

31853185
static int get_object_list_from_bitmap(struct rev_info *revs)
31863186
{
3187-
if (!(bitmap_git = prepare_bitmap_walk(revs)))
3187+
if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
31883188
return -1;
31893189

31903190
if (pack_options_allow_reuse() &&
@@ -3198,7 +3198,8 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
31983198
display_progress(progress_state, nr_result);
31993199
}
32003200

3201-
traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap);
3201+
traverse_bitmap_commit_list(bitmap_git, revs,
3202+
&add_object_entry_from_bitmap);
32023203
return 0;
32033204
}
32043205

@@ -3562,7 +3563,6 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
35623563
if (filter_options.choice) {
35633564
if (!pack_to_stdout)
35643565
die(_("cannot use --filter without --stdout"));
3565-
use_bitmap_index = 0;
35663566
}
35673567

35683568
/*

builtin/rev-list.c

Lines changed: 97 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,26 @@ static int finish_object(struct object *obj, const char *name, void *cb_data)
253253
static void show_object(struct object *obj, const char *name, void *cb_data)
254254
{
255255
struct rev_list_info *info = cb_data;
256+
struct rev_info *revs = info->revs;
257+
256258
if (finish_object(obj, name, cb_data))
257259
return;
258260
display_progress(progress, ++progress_counter);
259261
if (info->flags & REV_LIST_QUIET)
260262
return;
263+
264+
if (revs->count) {
265+
/*
266+
* The object count is always accumulated in the .count_right
267+
* field for traversal that is not a left-right traversal,
268+
* and cmd_rev_list() made sure that a .count request that
269+
* wants to count non-commit objects, which is handled by
270+
* the show_object() callback, does not ask for .left_right.
271+
*/
272+
revs->count_right++;
273+
return;
274+
}
275+
261276
if (arg_show_object_names)
262277
show_object_with_name(stdout, obj, name);
263278
else
@@ -364,6 +379,79 @@ static inline int parse_missing_action_value(const char *value)
364379
return 0;
365380
}
366381

382+
static int try_bitmap_count(struct rev_info *revs,
383+
struct list_objects_filter_options *filter)
384+
{
385+
uint32_t commit_count = 0,
386+
tag_count = 0,
387+
tree_count = 0,
388+
blob_count = 0;
389+
int max_count;
390+
struct bitmap_index *bitmap_git;
391+
392+
/* This function only handles counting, not general traversal. */
393+
if (!revs->count)
394+
return -1;
395+
396+
/*
397+
* A bitmap result can't know left/right, etc, because we don't
398+
* actually traverse.
399+
*/
400+
if (revs->left_right || revs->cherry_mark)
401+
return -1;
402+
403+
/*
404+
* If we're counting reachable objects, we can't handle a max count of
405+
* commits to traverse, since we don't know which objects go with which
406+
* commit.
407+
*/
408+
if (revs->max_count >= 0 &&
409+
(revs->tag_objects || revs->tree_objects || revs->blob_objects))
410+
return -1;
411+
412+
/*
413+
* This must be saved before doing any walking, since the revision
414+
* machinery will count it down to zero while traversing.
415+
*/
416+
max_count = revs->max_count;
417+
418+
bitmap_git = prepare_bitmap_walk(revs, filter);
419+
if (!bitmap_git)
420+
return -1;
421+
422+
count_bitmap_commit_list(bitmap_git, &commit_count,
423+
revs->tree_objects ? &tree_count : NULL,
424+
revs->blob_objects ? &blob_count : NULL,
425+
revs->tag_objects ? &tag_count : NULL);
426+
if (max_count >= 0 && max_count < commit_count)
427+
commit_count = max_count;
428+
429+
printf("%d\n", commit_count + tree_count + blob_count + tag_count);
430+
free_bitmap_index(bitmap_git);
431+
return 0;
432+
}
433+
434+
static int try_bitmap_traversal(struct rev_info *revs,
435+
struct list_objects_filter_options *filter)
436+
{
437+
struct bitmap_index *bitmap_git;
438+
439+
/*
440+
* We can't use a bitmap result with a traversal limit, since the set
441+
* of commits we'd get would be essentially random.
442+
*/
443+
if (revs->max_count >= 0)
444+
return -1;
445+
446+
bitmap_git = prepare_bitmap_walk(revs, filter);
447+
if (!bitmap_git)
448+
return -1;
449+
450+
traverse_bitmap_commit_list(bitmap_git, revs, &show_object_fast);
451+
free_bitmap_index(bitmap_git);
452+
return 0;
453+
}
454+
367455
int cmd_rev_list(int argc, const char **argv, const char *prefix)
368456
{
369457
struct rev_info revs;
@@ -521,8 +609,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
521609
if (revs.show_notes)
522610
die(_("rev-list does not support display of notes"));
523611

524-
if (filter_options.choice && use_bitmap_index)
525-
die(_("cannot combine --use-bitmap-index with object filtering"));
612+
if (revs.count &&
613+
(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
614+
(revs.left_right || revs.cherry_mark))
615+
die(_("marked counting is incompatible with --objects"));
526616

527617
save_commit_buffer = (revs.verbose_header ||
528618
revs.grep_filter.pattern_list ||
@@ -533,28 +623,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
533623
if (show_progress)
534624
progress = start_delayed_progress(show_progress, 0);
535625

536-
if (use_bitmap_index && !revs.prune) {
537-
if (revs.count && !revs.left_right && !revs.cherry_mark) {
538-
uint32_t commit_count;
539-
int max_count = revs.max_count;
540-
struct bitmap_index *bitmap_git;
541-
if ((bitmap_git = prepare_bitmap_walk(&revs))) {
542-
count_bitmap_commit_list(bitmap_git, &commit_count, NULL, NULL, NULL);
543-
if (max_count >= 0 && max_count < commit_count)
544-
commit_count = max_count;
545-
printf("%d\n", commit_count);
546-
free_bitmap_index(bitmap_git);
547-
return 0;
548-
}
549-
} else if (revs.max_count < 0 &&
550-
revs.tag_objects && revs.tree_objects && revs.blob_objects) {
551-
struct bitmap_index *bitmap_git;
552-
if ((bitmap_git = prepare_bitmap_walk(&revs))) {
553-
traverse_bitmap_commit_list(bitmap_git, &show_object_fast);
554-
free_bitmap_index(bitmap_git);
555-
return 0;
556-
}
557-
}
626+
if (use_bitmap_index) {
627+
if (!try_bitmap_count(&revs, &filter_options))
628+
return 0;
629+
if (!try_bitmap_traversal(&revs, &filter_options))
630+
return 0;
558631
}
559632

560633
if (prepare_revision_walk(&revs))

ewah/bitmap.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ void bitmap_set(struct bitmap *self, size_t pos)
5050
self->words[block] |= EWAH_MASK(pos);
5151
}
5252

53+
void bitmap_unset(struct bitmap *self, size_t pos)
54+
{
55+
size_t block = EWAH_BLOCK(pos);
56+
57+
if (block < self->word_alloc)
58+
self->words[block] &= ~EWAH_MASK(pos);
59+
}
60+
5361
int bitmap_get(struct bitmap *self, size_t pos)
5462
{
5563
size_t block = EWAH_BLOCK(pos);

ewah/ewok.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ struct bitmap {
174174
struct bitmap *bitmap_new(void);
175175
struct bitmap *bitmap_word_alloc(size_t word_alloc);
176176
void bitmap_set(struct bitmap *self, size_t pos);
177+
void bitmap_unset(struct bitmap *self, size_t pos);
177178
int bitmap_get(struct bitmap *self, size_t pos);
178179
void bitmap_reset(struct bitmap *self);
179180
void bitmap_free(struct bitmap *self);

object.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,15 @@ int object_list_contains(struct object_list *list, struct object *obj)
308308
return 0;
309309
}
310310

311+
void object_list_free(struct object_list **list)
312+
{
313+
while (*list) {
314+
struct object_list *p = *list;
315+
*list = p->next;
316+
free(p);
317+
}
318+
}
319+
311320
/*
312321
* A zero-length string to which object_array_entry::name can be
313322
* initialized without requiring a malloc/free.

object.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ struct object_list *object_list_insert(struct object *item,
151151

152152
int object_list_contains(struct object_list *list, struct object *obj);
153153

154+
void object_list_free(struct object_list **list);
155+
154156
/* Object array handling .. */
155157
void add_object_array(struct object *obj, const char *name, struct object_array *array);
156158
void add_object_array_with_path(struct object *obj, const char *name, struct object_array *array, unsigned mode, const char *path);

0 commit comments

Comments
 (0)