On Thu, Oct 31, 2024 at 02:43:19PM -0700, Jonathan Tan wrote: > > Another thought about this whole thing is that we essentially have a > > code path that says: "I found this object from the commit-graph, but > > don't know if I actually have it on disk, so mark it to be checked later > > via has_object()". > > > > I wonder if it would be more straightforward to replace the call to > > lookup_commit_in_graph() with a direct call to has_object() in the > > deref_without_lazy_fetch() function, which I think would both (a) > > eliminate the need for a new flag bit to be allocated, and (b) prevent > > looking up the object twice. > > > > Thoughts? > > This would undo the optimization in 62b5a35a33 (fetch-pack: optimize > loading of refs via commit graph, 2021-09-01), and also would not work > without changes to the fetch negotiation code - I tried to describe it > in the commit message, perhaps not very clearly, but the issue is that > even if we emit "want X", the fetch negotiation code would emit "have > X" (the X is the same in both), and at least for our JGit server at > $DAYJOB, the combination of "want X" and "have X" results in the server > sending an empty packfile (reasonable behavior, I think). (And I don't > think the changes to the fetch negotiation code are worth it.) Thanks for the clarifications above. What I was trying to poke at here was... doesn't the change as presented undo that optimization, just in a different way? In 62b5a35a33 we taught deref_without_lazy_fetch() to lookup commits through the commit-graph. But in this patch, we now call has_object() on top of that existing check. Am I missing something obvious? Thanks, Taylor
