On Mon, Apr 12, 2021 at 2:08 PM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > Here is the second patch series submission coming out of the sparse-index > RFC [1]. > > [1] > https://lore.kernel.org/git/pull.847.git.1611596533.gitgitgadget@xxxxxxxxx/ > > This is based on ds/sparse-index. > > The point of this series is to insert protections for the consumers of the > in-memory index to avoid unintended behavior change when using a sparse > index versus a full one. > > We mark certain regions of code as needing a full index, so we call > ensure_full_index() to expand a sparse index to a full one, if necessary. > These protections are inserted file-by-file in every loop over all cache > entries. Well, "most" loops, because some are going to be handled in the > very next series so I leave them out. > > Many callers use index_name_pos() to find a path by name. In these cases, we > can check if that position resolves to a sparse directory instance. In those > cases, we just expand to a full index and run the search again. > > The last few patches deal with the name-hash hashtable for doing O(1) > lookups. > > These protections don't do much right now, since the previous series created > the_repository->settings.command_requires_full_index to guard all index > reads and writes to ensure the in-memory copy is full for commands that have > not been tested with the sparse index yet. > > However, after this series is complete, we now have a straight-forward plan > for making commands "sparse aware" one-by-one: > > 1. Disable settings.command_requires_full_index to allow an in-memory > sparse-index. > 2. Run versions of that command under a debugger, breaking on > ensure_full_index(). > 3. Examine the call stack to determine the context of that expansion, then > implement the proper behavior in those locations. > 4. Add tests to ensure we are checking this logic in the presence of sparse > directory entries. > > I will admit that mostly it is the writing of the test cases that takes the > most time in the conversions I've done so far. > > > Updates in v3 > ============= > > * I updated based on Elijah's feedback. > * One new patch splits out a change that Elijah (rightfully) pointed out > did not belong with the patch it was originally in. > > I gave it time to see if any other comments came in, but it looks like > review stabilized. I probably waited a bit longer than I should have. This round looks good to me. Reviewed-by: Elijah Newren <newren@xxxxxxxxx>
