From: Derrick Stolee Date: Mon, 22 Jun 2026 20:28:22 GMT Subject: Re: [PATCH/RFC 4/6] t6600: add test cases for side-exhaustion edge cases Message-ID: <80a0426c-7146-4212-a8cd-d884f4424b2c@gmail.com> In-Reply-To: On 6/22/2026 3:25 PM, Kristofer Karlsson wrote: > On Mon, 22 Jun 2026 at 20:15, Derrick Stolee wrote: >> It's usually my preference to see these tests show up before the >> new code arrives, that way we can see that they already work with >> the old logic and continue to work with the new logic. >> >> It's minor, but putting them after your code change may be adding >> enforcement of a change of behavior. > > Agreed, I actually also prefer that in practice so I am not > sure why I ordered them this way - perhaps some attempt at > making it easier to review (show the idea and change before > the verification). I will reorder to put all new tests as the first commit > (or second, if I will also introduce a status-quo technical first). > >> >> One thing that could be helpful here is to consider tracing a >> count of "commits walked" in the merge-base code, then you could >> have these tests demonstrate the performance benefit by checking >> for that number changing. > > Good idea, I actually had some of that locally when developing it, > but I removed the ugly traces before submitting this. I will try to > re-introduce that in a nice way. It would be neat to let tests > inspect that side effect, though in the worst case that could make > it fragile. At the very least it's good for human debugging though. And to be clear, I'm suggesting using trace2_data_intmax() calls to get structured data that can be parsed in the GIT_TRACE2_EVENT logs during tests. It could also be picked up by teletry tools that listen to trace2 output, if desired. It will show up differently in GIT_TRACE2_PERF, but that's a nice human-readable way to debug things. >> In t6600, that tracing number would not be the same across the >> three different data shapes (full graph, half graph, no graph) and >> that could be valuable to demonstrate in tests. > > Agreed, the number of commits visited would be more interesting > than the relative performance numbers since it's an algorithmic > change rather than a micro-optimization. They are both interesting, but only the commit count can be guaranteed rigorously in the test suite. It's possible that a great improvement to such a trace doesn't result in great end-to- end time improvement, but I believe that it is true in this case. Thanks, -Stolee