On Tue, Jan 12, 2021 at 04:37:09AM -0500, Jeff King wrote: > This definitely makes sense in the long run. I could take or leave it as > a final patch in _this_ series (as opposed to the first patch in a > subsequent series adding the rev files). > > > do { > > const unsigned mi = lo + (hi - lo) / 2; > > - if (revindex[mi].offset == ofs) { > > + off_t got = pack_pos_to_offset(p, mi); > > > They're both constant-time, so performance should be the same big-O. The > function has extra BUG() checks. I doubt those are measurable in > practice, though. Funny enough, I have moved this patch between the two so many times before submitting this. I tend to agree that I don't think it makes a difference in which series this patch goes, so I'm just as happy to leave it where it is and stop thinking about it ;-). If others have strong feelings, this can be dropped when queuing and I'll send it along as the first commit in the second series (which will have to be updated along with this one). > -Peff Thanks, Taylor