On Fri, Aug 20, 2021 at 10:47:11AM -0400, Derrick Stolee wrote: > On 8/20/2021 6:08 AM, Patrick Steinhardt wrote: > > When fetching refs, we are doing two connectivity checks: > > > > - The first one in `fetch_refs()` is done such that we can > > short-circuit the case where we already have all objects > > referenced by the updated set of refs. > > > > - The second one in `store_updated_refs()` does a sanity check that > > we have all objects after we have fetched the packfile. > > > > We always execute both connectivity checks, but this is wasteful in case > > the first connectivity check already notices that we have all objects > > locally available. > > > > Refactor the code to do both connectivity checks in `fetch_refs()`, > > which allows us to easily skip the second connectivity check if we > > already have all objects available. This refactoring is safe to do given > > that we always call `fetch_refs()` followed by `consume_refs()`, which > > is the only caller of `store_updated_refs()`. > > Should we try to make it more clear that fetch_refs() must be followed > by consume_refs() via a comment above the fetch_refs(), or possibly even > its call sites? I wasn't quite happy with this outcome, either. How about we instead merge both functions into `fetch_and_consume_refs()`? Both are quite short, and that makes sure we always call them together to make the requirement explicit. I'll add another patch to do this refactoring. Patrick
Attachment:
signature.asc
Description: PGP signature
