Thread (6 messages) flat view 6 messages, 2 authors, 2022-09-06

Re: [PATCH 1/2] lookup_commit_in_graph(): use prepare_commit_graph() to check for graph

From: Taylor Blau <hidden>
Date: 2022-09-06 21:17:01

On Tue, Sep 06, 2022 at 05:02:13PM -0400, Jeff King wrote:
I couldn't find any other reason to avoid calling prepare_commit_graph()
here (especially since it ends up lazy-loaded as discussed above). The
cost of the call should not be high; after the first call, it is
simplified down to a few integer checks.
Neither could I. Obviously f559d6d45e (revision: avoid hitting packfiles
when commits are in commit-graph, 2021-08-09) is adding a call to look
for a commit by object ID in the commit-graph where there wasn't one
before, so there isn't anything to compare to there.

But the next-closest function `load_commit_graph_info()` calls
`prepare_commit_graph()` (via `repo_find_commit_pos_in_graph()`).

And that all matches my understanding that `r->objects->commit_graph` is
lazily loaded. Perhaps it should be made more difficult to access via
the struct member, and instead done behind a function like
`prepare_commit_graph()` (modified to return the `struct commit_graph*`
if one was found).

But that's for another day :-). This is obviously correct in the
meantime.

Thanks,
Taylor
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help