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? Thanks, -Stolee
