Re: [PATCH v2 12/24] pack-bitmap-write: fill bitmap with commit history

> 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

>     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?