On 4/10/2023 6:53 PM, Taylor Blau wrote: > In the vast majority of cases, this trade-off favors the on-disk ".rev" > files. But in certain cases, the in-memory variant performs more > favorably. Since these cases are narrow, and performance is machine- and > repository-dependent, this series also introduces a new configuration > option to disable reading ".rev" files in the third commit. I agree that the performance trade-off indicates that having the .rev files is preferred. It makes operations that _can_ be very fast as fast as possible (inspecting a small number of objects is much faster because we don't generate the in-memory index in O(N) time) and you create a knob for disabling it in the case that we are already doing something that inspects almost-all objects. > The series is structured as follows: > > - A couple of cleanup patches to plug a leak in stage_tmp_packfiles(). > - Three patches to enable `pack.readReverseIndex`. > - Another patch to change the default of `pack.writeReverseIndex` from > "false" to "true". > - A final patch to enable the test suite to be run in a mode that does > not use on-disk ".rev" files. This was an easy series to read. I applied the patches locally and poked around in the resulting code as I went along. This led to a couple places where I recommend a few changes, including a new patch that wires repository pointers through a few more method layers. Thanks, -Stolee
