On 11/22/2020 4:50 PM, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> >> The fill_bitmap_commit() method assumes that every parent of the given >> commit is already part of the current bitmap. Instead of making that >> assumption, let's walk parents until we reach commits already part of >> the bitmap. Set the value for that parent immediately after querying to >> save time doing double calls to find_object_pos() and to avoid inserting >> the parent into the queue multiple times. > > Is it because somebody found a case where the assumption does not > hold and the code with the assumption produces a wrong result? Is > it because we can get a better result without making the assumption > the current code does? The algorithm from "pack-bitmap-write: reimplement bitmap writing" that calls fill_bitmap_commit() satisfies this assumption, since it computes a reachability bitmap for every commit during the reverse walk. We will soon change that algorithm to "skip" commits, so we need this step in fill_bitmap_commit() to walk forward to fill the gaps. > In other words, can we explain why we are making the change in the > proposed log message? I'm sure Taylor and I can work out a better wording to make this more clear. Thanks, -Stolee