Skip to content

ls-files: conditionally leave index sparse #1955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

derrickstolee
Copy link

Here's a small sparse index performance update based on a user report.

Thanks,
-Stolee

cc: [email protected]

When running 'git ls-files' with a pathspec, the index entries get
filtered according to that pathspec before iterating over them in
show_files().  In 7808709 (ls-files: add --sparse option,
2021-12-22), this iteration was prefixed with a check for the '--sparse'
option which allows the command to output directory entries; this
created a pre-loop call to ensure_full_index().

However, when a user runs 'git ls-files' where the pathspec matches
directories that are recursively matched in the sparse-checkout, there
are not any sparse directories that match the pathspec so they would not
be written to the output. The expansion in this case is just a
performance drop for no behavior difference.

Replace this global check to expand the index with a check inside the
loop for a matched sparse directory. If we see one, then expand the
index and continue from the current location. This is safe since the
previous entries in the index did not have any sparse directories and
thus would remain stable in this expansion.

A test in t1092 confirms that this changes the behavior.

Signed-off-by: Derrick Stolee <[email protected]>
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

* alone.
*/
ensure_full_index(repo->index);
ce = repo->index->cache[i];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering whether we'd want to avoid calling ensure_full_index() multiple times, but it seems that it returns early if istate->sparse_index == INDEX_EXPANDED, so it's safe.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But also we will never have S_ISSPARSEDIR() true after expansion.

@derrickstolee
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Aug 15, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1955/derrickstolee/ls-files-sparse-index-v1

To fetch this version to local tag pr-1955/derrickstolee/ls-files-sparse-index-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1955/derrickstolee/ls-files-sparse-index-v1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants