> Just because a commit happened to be bitmapped last time does not make > it a good candidate for having a bitmap this time. In particular, we may > choose bitmaps based on how recent they are in history, or whether a ref > tip points to them, and those things will change. We're better off > re-considering fresh which commits are good candidates. > > Reusing the existing bitmap _is_ a reasonable thing to do to save > computation. But only reusing exact bitmaps is a weak form of this. If > we have an old bitmap for A and now want a new bitmap for its child, we > should be able to compute that only by looking at trees and that are new > to the child. But this code would consider only exact reuse (which is > perhaps why it was eager to select those commits in the first place). Makes sense. > -int rebuild_existing_bitmaps(struct bitmap_index *bitmap_git, > - struct packing_data *mapping, > - kh_oid_map_t *reused_bitmaps, > - int show_progress) > +uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git, > + struct packing_data *mapping) [snip body of function] Here, a lot of the function is deleted, and only the part that creates the mapping from old indices to new indices remains - hence, the renaming of the function. OK. Overall this looks good. I was wondering if there would be any functions now unused, but looking at the deleted lines, that doesn't seem to be the case.
