> Thanks. Stolee and I worked a little on revising this last week, and I > think that the current log message is more along these lines. Here's > what we wrote: > > pack-bitmap-write: fill bitmap with commit history > > The current implementation of bitmap_writer_build() creates a > reachability bitmap for every walked commit. After computing a bitmap > for a commit, those bits are pushed to an in-progress bitmap for its > children. > > fill_bitmap_commit() assumes the bits corresponding to objects > reachable from the parents of a commit are already set. This means that > when visiting a new commit, we only have to walk the objects reachable > between it and any of its parents. > > A future change to bitmap_writer_build() will relax this condition so > not all parents have their reachable objects set in the in-progress I would write "not all parents have their bits set" instead, but this is fine too. > bitmap. Prepare for that by having 'fill_bitmap_commit()' walk > parents until reaching commits whose bits are already set. Then, walk > the trees for these commits as well. > > This has no functional change with the current implementation of > bitmap_writer_build(). > > > > static void fill_bitmap_commit(struct bb_commit *ent, > > > - struct commit *commit) > > > + struct commit *commit, > > > + struct prio_queue *queue) > > > > As far as I can see, this function expects an empty queue and always > > ends with the queue empty, and the only reason why we don't instantiate > > a new queue every time is so that we can save on the internal array > > allocation/deallocation. Maybe add a comment to that effect. > > Sure. Would you find a comment like that more helpful above > 'fill_bitmap_commit()', or above the declaration of 'queue' (in > 'bitmap_writer_build()') itself? I think it's better with fill_bitmap_commit(), as it's the one in control of how "queue" will be used.
