On Fri, Mar 24, 2023 at 11:06:55PM +0100, Oswald Buddenhagen wrote: > > Ah, that is your problem. When "replace" refs are in use, the data > > stored in the commit-graph can't reliably be used. [...] > > > why isn't the commit-graph built with the replaces applied (and tagged by a > hash of the used replaces, so we know when to ignore it)? I think a similar idea has come up before, but we decided to do the simplest safe thing (disabling optimizations) to start with. And then somebody who really cared about making optimizations work with commit graphs could come along and do so later. Nobody has yet; you could be that someone. ;) > at minimum, i'd expect a warning giving a reason when the graph is ignored. That might be reasonable. The commit graph is an optimization, so we'd never produce a wrong answer by ignoring it. And since the fallback was the status quo before the optimizations were implemented, it didn't seem like that big a deal. But these days the performance many of us expect is with those optimizations, so perhaps the tables have turned. I do think there might be some complications, though. I think we may build commit graphs by default these days during "gc" and even incrementally after "fetch". If we warned when the graphs are disabled, it basically means that every command in a repo with replace refs would issue the warning. > > I'd still be curious to see the > > difference between "just commit graphs" and "commit graphs plus the > > patch I showed earlier". I think it should make things faster, but if > > it's only a few milliseconds on average, it's not that urgent to pursue. > > > if there is a speed difference at all, it gets drowned out by the noise. OK, thanks for testing. I do think that looking into a true single traversal might make sense, but I don't think we've seen a case yet where it's a substantial speedup. -Peff
