Re: [PATCH 3/3] parse_object(): check commit-graph when skip_hash set
On 9/6/2022 7:06 PM, Jeff King wrote:
Show 8 quoted lines
> 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().
Show 8 quoted lines
> 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.
Show 7 quoted lines
> - 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