On 9/6/2022 7:06 PM, Jeff King wrote: > If the caller told us that they don't care about us checking the object > hash, then we're free to implement any optimizations that get us the > parsed value more quickly. An obvious one is to check the commit graph > before loading an object from disk. And in fact, both of the callers who > pass in this flag are already doing so before they call parse_object()! > > So we can simplify those callers, as well as any possible future ones, > by moving the logic into parse_object(). Nice! > There are two subtle things to note in the diff, but neither has any > impact in practice: > > - it seems least-surprising here to do the graph lookup on the > git-replace'd oid, rather than the original. This is in theory a > change of behavior from the earlier code, as neither caller did a > replace lookup itself. But in practice it doesn't matter, as we > disable the commit graph entirely if there are any replace refs. I can confirm that this is the case. > - the caller in get_reference() passes the skip_hash flag only if > revs->verify_objects isn't set, whereas it would look in the commit > graph unconditionally. In practice this should not matter as we > should disable the commit graph entirely when using verify_objects > (and that was done recently in another patch). > > So this should be a pure cleanup with no behavior change. Excellent, thanks! -Stolee
