{"thread":{"id":"63493","subject":"[PATCH] name-hash: don't add sparse directories in threaded lazy init","startedAt":"2025-05-21T11:40:48Z","lastAt":"2025-05-22T01:48:51Z","messageCount":13,"participants":["Alex Mironov via GitGitGadget","Derrick Stolee","Junio C Hamano","Alex Mironov"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"518558","messageId":"pull.1970.git.git.1747827645129.gitgitgadget@gmail.com","threadId":"63493","inReplyTo":null,"subject":"[PATCH] name-hash: don't add sparse directories in threaded lazy init","fromName":"Alex Mironov via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-21T11:40:44Z","receivedAt":"2025-05-21T11:40:48Z","isPatch":true,"sender":{"key":"alexandrfox@gmail.com","avatar":"https://avatars.githubusercontent.com/u/199416?v=4"},"body":"From: Alex Mironov \n\nSimilarly to 5f116695864788d1fe45ff06bfad7a71a8d98d0a\nmake sure to avoid placing sparse directories into the name_hash\nhashtable whenever multithreaded initialization is performed.\n\nSparse directory entries represent a directory that is outside the\nsparse-checkout definition. These are not paths to blobs, so should not\nbe added to the name_hash table as they must never be queried.\n\nSigned-off-by: Alex Mironov \n---\n name-hash: don't add sparse directories in threaded lazy init\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1970%2Falexandrfox%2Ffix-threaded-hash-name-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1970/alexandrfox/fix-threaded-hash-name-v1\nPull-Request: https://github.com/git/git/pull/1970\n\n name-hash.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex d66de1cdfd5..03123a8779a 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -492,6 +492,9 @@ static void *lazy_name_thread_proc(void *_data)\n \tfor (k = 0; k < d->istate->cache_nr; k++) {\n \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n \t\tce_k->ce_flags |= CE_HASHED;\n+\t\tif (S_ISSPARSEDIR(ce_k->ce_mode)) {\n+\t\t\tcontinue;\n+\t\t}\n \t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n \t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n \t}\n\nbase-commit: 8613c2bb6cd16ef530dc5dd74d3b818a1ccbf1c0\n-- \ngitgitgadget\n"},{"id":"518603","messageId":"9c26d844-6ac5-449b-a5ff-a842ed6ba8b9@gmail.com","threadId":"63493","inReplyTo":"pull.1970.git.git.1747827645129.gitgitgadget@gmail.com","subject":"Re: [PATCH] name-hash: don't add sparse directories in threaded lazy init","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-21T17:17:08Z","receivedAt":"2025-05-21T17:17:10Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/21/2025 7:40 AM, Alex Mironov via GitGitGadget wrote:\n> From: Alex Mironov \n> \n> Similarly to 5f116695864788d1fe45ff06bfad7a71a8d98d0a\n\nnit: we typically use the \"reference\" style to refer to other\ncommits, use 'git log -1 --pretty=reference ' to get output\nlike this:\n\n 5f116695864 (name-hash: don't add directories to name_hash, 2021-04-12)\n\n> make sure to avoid placing sparse directories into the name_hash\n> hashtable whenever multithreaded initialization is performed.\n> \n> Sparse directory entries represent a directory that is outside the\n> sparse-checkout definition. These are not paths to blobs, so should not\n> be added to the name_hash table as they must never be queried.\n> \n> Signed-off-by: Alex Mironov \n> ---\n> name-hash: don't add sparse directories in threaded lazy init\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1970%2Falexandrfox%2Ffix-threaded-hash-name-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1970/alexandrfox/fix-threaded-hash-name-v1\n> Pull-Request: https://github.com/git/git/pull/1970\n> \n> name-hash.c | 3 +++\n> 1 file changed, 3 insertions(+)\n> \n> diff --git a/name-hash.c b/name-hash.c\n> index d66de1cdfd5..03123a8779a 100644\n> --- a/name-hash.c\n> +++ b/name-hash.c\n> @@ -492,6 +492,9 @@ static void *lazy_name_thread_proc(void *_data)\n> \tfor (k = 0; k < d->istate->cache_nr; k++) {\n> \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n> \t\tce_k->ce_flags |= CE_HASHED;\n> +\t\tif (S_ISSPARSEDIR(ce_k->ce_mode)) {\n> +\t\t\tcontinue;\n> +\t\t}\n\nnit: for one-line blocks, we usually skip the braces. But I think\nthat it might be better to reverse the logic to get something like:\n\n\tif (!S_ISSPARSEDIR(ce_k->ce_mode) {\n \t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n \t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n\t}\n\nThis seems to be a performance-only fix, and it might be interesting\nto see if there is any impact on p2000-sparse-operations.sh. Those\ntests don't focus on many sparse-directory entries, so that may not\ndemonstrate any meaningful difference.\n\nThanks,\n-Stolee\n\n"},{"id":"518604","messageId":"xmqqwma9ak7p.fsf@gitster.g","threadId":"63493","inReplyTo":"9c26d844-6ac5-449b-a5ff-a842ed6ba8b9@gmail.com","subject":"Re: [PATCH] name-hash: don't add sparse directories in threaded lazy init","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T18:32:10Z","receivedAt":"2025-05-21T18:32:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> This seems to be a performance-only fix, ...\n\nHmph, then the proposed log message is phrased in a slightly\nmisleading way. I took that \"should not\" below ...\n\n> Sparse directory entries represent a directory that is outside the\n> sparse-checkout definition. These are not paths to blobs, so should not\n> be added to the name_hash table as they must never be queried.\n\n... to be saying that we would find something we should not find\nwhen querying the name_hash table if we didn't omit them, and the\nchange is done for correctness.\n\nThanks for a quick review. Very much appreciated.\n\n"},{"id":"518610","messageId":"CAC97EbwMKB5MUD==-6dHuj18OGa--p_UzWw+CsbGTM0iQ8Mw0A@mail.gmail.com","threadId":"63493","inReplyTo":"9c26d844-6ac5-449b-a5ff-a842ed6ba8b9@gmail.com","subject":"Re: [PATCH] name-hash: don't add sparse directories in threaded lazy init","fromName":"Alex Mironov","fromEmail":"alexandrfox@gmail.com","sentAt":"2025-05-21T20:07:04Z","receivedAt":"2025-05-21T20:07:20Z","isPatch":true,"sender":{"key":"alexandrfox@gmail.com","avatar":"https://avatars.githubusercontent.com/u/199416?v=4"},"body":"Hey Derrick,\n\nAppreciate your quick response! I addressed the nits, and will submit\nthe new patch shortly.\n\n> This seems to be a performance-only fix\n\nIndeed, and it's more impactful for the specific setup we have inside\nour org. In short, we have a custom implementation of the sparse\ncheckout logic that makes sure to leave files outside of sparse cones\npresent on disk. Because of this, git with sparse index enabled\nfrequently resorted to a full index expansion despite\n'sparse.expectFilesOutsideOfPatterns=true' config set which, I assume,\nwas only made working with the full index. This problem specifically\nwas tricky to catch since multithreaded lazy load logic applies only\nafter a certain limit of objects present in the index.\n\nI hope I will be able to send more patches soon to fix-up bits and\npieces of the 'sparse.expectFilesOutsideOfPatterns' flag with respect\nto other expansions. I'm planning to guard those under the same\nconfiguration option, let me know if you think it'd be better to\nintroduce a new option.\n\n\nOn Wed, May 21, 2025 at 7:17 PM Derrick Stolee wrote:\n>\n> On 5/21/2025 7:40 AM, Alex Mironov via GitGitGadget wrote:\n> > From: Alex Mironov \n> >\n> > Similarly to 5f116695864788d1fe45ff06bfad7a71a8d98d0a\n>\n> nit: we typically use the \"reference\" style to refer to other\n> commits, use 'git log -1 --pretty=reference ' to get output\n> like this:\n>\n> 5f116695864 (name-hash: don't add directories to name_hash, 2021-04-12)\n>\n> > make sure to avoid placing sparse directories into the name_hash\n> > hashtable whenever multithreaded initialization is performed.\n> >\n> > Sparse directory entries represent a directory that is outside the\n> > sparse-checkout definition. These are not paths to blobs, so should not\n> > be added to the name_hash table as they must never be queried.\n> >\n> > Signed-off-by: Alex Mironov \n> > ---\n> > name-hash: don't add sparse directories in threaded lazy init\n> >\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1970%2Falexandrfox%2Ffix-threaded-hash-name-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1970/alexandrfox/fix-threaded-hash-name-v1\n> > Pull-Request: https://github.com/git/git/pull/1970\n> >\n> > name-hash.c | 3 +++\n> > 1 file changed, 3 insertions(+)\n> >\n> > diff --git a/name-hash.c b/name-hash.c\n> > index d66de1cdfd5..03123a8779a 100644\n> > --- a/name-hash.c\n> > +++ b/name-hash.c\n> > @@ -492,6 +492,9 @@ static void *lazy_name_thread_proc(void *_data)\n> > for (k = 0; k < d->istate->cache_nr; k++) {\n> > struct cache_entry *ce_k = d->istate->cache[k];\n> > ce_k->ce_flags |= CE_HASHED;\n> > + if (S_ISSPARSEDIR(ce_k->ce_mode)) {\n> > + continue;\n> > + }\n>\n> nit: for one-line blocks, we usually skip the braces. But I think\n> that it might be better to reverse the logic to get something like:\n>\n> if (!S_ISSPARSEDIR(ce_k->ce_mode) {\n> hashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n> hashmap_add(&d->istate->name_hash, &ce_k->ent);\n> }\n>\n> This seems to be a performance-only fix, and it might be interesting\n> to see if there is any impact on p2000-sparse-operations.sh. Those\n> tests don't focus on many sparse-directory entries, so that may not\n> demonstrate any meaningful difference.\n>\n> Thanks,\n> -Stolee\n>\n\n\n-- \nBest,\nAlex Mironov\n"},{"id":"518611","messageId":"pull.1970.v2.git.git.1747858585623.gitgitgadget@gmail.com","threadId":"63493","inReplyTo":"pull.1970.git.git.1747827645129.gitgitgadget@gmail.com","subject":"[PATCH v2] name-hash: don't add sparse directories in threaded lazy init","fromName":"Alex Mironov via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-21T20:16:25Z","receivedAt":"2025-05-21T20:16:28Z","isPatch":true,"sender":{"key":"alexandrfox@gmail.com","avatar":"https://avatars.githubusercontent.com/u/199416?v=4"},"body":"From: Alex Mironov \n\nEnsure that logic added in 5f11669586 (name-hash: don't add directories\nto name_hash, 2021-04-12) also applies in multithreaded hashtable init\npath.\n\nSparse directory entries represent a directory that is outside the\nsparse-checkout definition. These are not paths to blobs, so should not\nbe added to the name_hash table as they must never be queried.\n\nSigned-off-by: Alex Mironov \n---\n name-hash: don't add sparse directories in threaded lazy init\n \n Changes since v1:\n \n * addressed feedback (code-style)\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1970%2Falexandrfox%2Ffix-threaded-hash-name-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1970/alexandrfox/fix-threaded-hash-name-v2\nPull-Request: https://github.com/git/git/pull/1970\n\nRange-diff vs v1:\n\n 1: d12ebc612c2 ! 1: fb378147c73 name-hash: don't add sparse directories in threaded lazy init\n @@ Metadata\n ## Commit message ##\n name-hash: don't add sparse directories in threaded lazy init\n \n - Similarly to 5f116695864788d1fe45ff06bfad7a71a8d98d0a\n - make sure to avoid placing sparse directories into the name_hash\n - hashtable whenever multithreaded initialization is performed.\n + Ensure that logic added in 5f11669586 (name-hash: don't add directories\n + to name_hash, 2021-04-12) also applies in multithreaded hashtable init\n + path.\n \n Sparse directory entries represent a directory that is outside the\n sparse-checkout definition. These are not paths to blobs, so should not\n @@ name-hash.c: static void *lazy_name_thread_proc(void *_data)\n \tfor (k = 0; k < d->istate->cache_nr; k++) {\n \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n \t\tce_k->ce_flags |= CE_HASHED;\n -+\t\tif (S_ISSPARSEDIR(ce_k->ce_mode)) {\n -+\t\t\tcontinue;\n +-\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n +-\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n ++\t\tif (!S_ISSPARSEDIR(ce_k->ce_mode)) {\n ++\t\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n ++\t\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n +\t\t}\n - \t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n - \t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n \t}\n + \n + \treturn NULL;\n\n\n name-hash.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex d66de1cdfd5..b91e2762678 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -492,8 +492,10 @@ static void *lazy_name_thread_proc(void *_data)\n \tfor (k = 0; k < d->istate->cache_nr; k++) {\n \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n \t\tce_k->ce_flags |= CE_HASHED;\n-\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n-\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n+\t\tif (!S_ISSPARSEDIR(ce_k->ce_mode)) {\n+\t\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n+\t\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n+\t\t}\n \t}\n \n \treturn NULL;\n\nbase-commit: 8613c2bb6cd16ef530dc5dd74d3b818a1ccbf1c0\n-- \ngitgitgadget\n"},{"id":"518613","messageId":"xmqqecwhaemu.fsf@gitster.g","threadId":"63493","inReplyTo":"pull.1970.v2.git.git.1747858585623.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] name-hash: don't add sparse directories in threaded lazy init","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T20:32:41Z","receivedAt":"2025-05-21T20:32:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Mironov via GitGitGadget\" writes:\n\n> From: Alex Mironov \n>\n> Ensure that logic added in 5f11669586 (name-hash: don't add directories\n> to name_hash, 2021-04-12) also applies in multithreaded hashtable init\n> path.\n>\n> Sparse directory entries represent a directory that is outside the\n> sparse-checkout definition. These are not paths to blobs, so should not\n> be added to the name_hash table as they must never be queried.\n\nThe second paragraph sounds as if this is a correctness fix,\ni.e. \"should not be added\" hints that we would see a wrong result\nreturned from the hashmap if you added these entries to the\nname_hash. If this is a performance-only fix, that should be more\nclearly stated. Also, can we have performance numbers in the\nproposed log message as well, or is the improvement too small to\nmeasure?\n\nThanks.\n"},{"id":"518615","messageId":"CAC97EbxRpG1ecWcQ=yJHnqe7gXYG7BNo1sF9e9Kf-EOkDySfpw@mail.gmail.com","threadId":"63493","inReplyTo":"xmqqecwhaemu.fsf@gitster.g","subject":"Re: [PATCH v2] name-hash: don't add sparse directories in threaded lazy init","fromName":"Alex Mironov","fromEmail":"alexandrfox@gmail.com","sentAt":"2025-05-21T20:37:58Z","receivedAt":"2025-05-21T20:38:13Z","isPatch":true,"sender":{"key":"alexandrfox@gmail.com","avatar":"https://avatars.githubusercontent.com/u/199416?v=4"},"body":"Hey Junio,\n\nWith respect to messaging I more or less copy-pasted Derricks message\nfrom the original commit for non-threaded init: please check the\nreferenced commit. Let me know if another wording is needed/preferred.\n\n> Also, can we have performance numbers in the\nproposed log message as well, or is the improvement too small to\nmeasure?\n\nIn their current form benchmarks do not show major improvements\noutside of special casing I mentioned in my previous message\n(existence of files/objects outside of sparse cones in special VFS\nsetups). So, for the general audience this indeed can be treated as a\ncorrectness fix.\n\nOn Wed, May 21, 2025 at 10:32 PM Junio C Hamano wrote:\n>\n> \"Alex Mironov via GitGitGadget\" writes:\n>\n> > From: Alex Mironov \n> >\n> > Ensure that logic added in 5f11669586 (name-hash: don't add directories\n> > to name_hash, 2021-04-12) also applies in multithreaded hashtable init\n> > path.\n> >\n> > Sparse directory entries represent a directory that is outside the\n> > sparse-checkout definition. These are not paths to blobs, so should not\n> > be added to the name_hash table as they must never be queried.\n>\n> The second paragraph sounds as if this is a correctness fix,\n> i.e. \"should not be added\" hints that we would see a wrong result\n> returned from the hashmap if you added these entries to the\n> name_hash. If this is a performance-only fix, that should be more\n> clearly stated. Also, can we have performance numbers in the\n> proposed log message as well, or is the improvement too small to\n> measure?\n>\n> Thanks.\n\n\n\n-- \nBest,\nAlex Mironov\n"},{"id":"518617","messageId":"xmqqo6vl8y81.fsf@gitster.g","threadId":"63493","inReplyTo":"CAC97EbxRpG1ecWcQ=yJHnqe7gXYG7BNo1sF9e9Kf-EOkDySfpw@mail.gmail.com","subject":"Re: [PATCH v2] name-hash: don't add sparse directories in threaded lazy init","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T21:12:30Z","receivedAt":"2025-05-21T21:12:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Mironov writes:\n\n> Hey Junio,\n>\n> With respect to messaging I more or less copy-pasted Derricks message\n> from the original commit for non-threaded init: please check the\n> referenced commit. Let me know if another wording is needed/preferred.\n\nI know what you did. Copying and pasting others fuzzy words into\nyour commit log message does not make your commit log message clear.\n\nI already said the given message is less clear than desired, so do I\nstill have to let you know???\n"},{"id":"518618","messageId":"xmqqjz698xqb.fsf@gitster.g","threadId":"63493","inReplyTo":"xmqqo6vl8y81.fsf@gitster.g","subject":"Re: [PATCH v2] name-hash: don't add sparse directories in threaded lazy init","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T21:23:08Z","receivedAt":"2025-05-21T21:23:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Alex Mironov writes:\n>\n>> Hey Junio,\n>>\n>> With respect to messaging I more or less copy-pasted Derricks message\n>> from the original commit for non-threaded init: please check the\n>> referenced commit. Let me know if another wording is needed/preferred.\n>\n> I know what you did. Copying and pasting others fuzzy words into\n> your commit log message does not make your commit log message clear.\n>\n> I already said the given message is less clear than desired, so do I\n> still have to let you know???\n\nActually after re-reading what Derrick wrote in that commit, I\nnotice that you didn't even copy-pasted his message in full. Here\nis the message in 5f116695 (name-hash: don't add directories to\nname_hash, 2021-04-12):\n\n name-hash: don't add directories to name_hash\n \n Sparse directory entries represent a directory that is outside the\n sparse-checkout definition. These are not paths to blobs, so should not\n be added to the name_hash table. Instead, they should be added to the\n directory hashtable when 'ignore_case' is true.\n \n Add a condition to avoid placing sparse directories into the name_hash\n hashtable. This avoids filling the table with extra entries that will\n never be queried.\n\nNotice that the second paragraph here makes it clear that how extra\nentries would not contribute to or hurt the correctness? You failed\nto copy-paste that crucial bit, which ended up making your version\nof the explanation much less clear why the change would not affect\ncorrectness than it could have been.\n"},{"id":"518620","messageId":"pull.1970.v3.git.git.1747862971672.gitgitgadget@gmail.com","threadId":"63493","inReplyTo":"pull.1970.v2.git.git.1747858585623.gitgitgadget@gmail.com","subject":"[PATCH v3] name-hash: don't add sparse directories in threaded lazy init","fromName":"Alex Mironov via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-21T21:29:31Z","receivedAt":"2025-05-21T21:29:35Z","isPatch":true,"sender":{"key":"alexandrfox@gmail.com","avatar":"https://avatars.githubusercontent.com/u/199416?v=4"},"body":"From: Alex Mironov \n\nEnsure that logic added in 5f11669586 (name-hash: don't add directories\nto name_hash, 2021-04-12) also applies in multithreaded hashtable init\npath.\n\nAs per the original single-threaded change above: sparse directory entries\nrepresent a directory that is outside the sparse-checkout definition.\nThese are not paths to blobs, so should not be added to the name_hash\ntable. Instead, they should be added to the directory hashtable when\n'ignore_case' is true.\n\nAdd a condition to avoid placing sparse directories into the name_hash\nhashtable. This avoids filling the table with extra entries that will\nnever be queried.\n\nSigned-off-by: Alex Mironov \n---\n name-hash: don't add sparse directories in threaded lazy init\n \n Changes since v2:\n \n * addressed commit message structure\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1970%2Falexandrfox%2Ffix-threaded-hash-name-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1970/alexandrfox/fix-threaded-hash-name-v3\nPull-Request: https://github.com/git/git/pull/1970\n\nRange-diff vs v2:\n\n 1: fb378147c73 ! 1: b91ccb640c5 name-hash: don't add sparse directories in threaded lazy init\n @@ Commit message\n to name_hash, 2021-04-12) also applies in multithreaded hashtable init\n path.\n \n - Sparse directory entries represent a directory that is outside the\n - sparse-checkout definition. These are not paths to blobs, so should not\n - be added to the name_hash table as they must never be queried.\n + As per the original single-threaded change above: sparse directory entries\n + represent a directory that is outside the sparse-checkout definition.\n + These are not paths to blobs, so should not be added to the name_hash\n + table. Instead, they should be added to the directory hashtable when\n + 'ignore_case' is true.\n +\n + Add a condition to avoid placing sparse directories into the name_hash\n + hashtable. This avoids filling the table with extra entries that will\n + never be queried.\n \n Signed-off-by: Alex Mironov \n \n\n\n name-hash.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/name-hash.c b/name-hash.c\nindex d66de1cdfd5..b91e2762678 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -492,8 +492,10 @@ static void *lazy_name_thread_proc(void *_data)\n \tfor (k = 0; k < d->istate->cache_nr; k++) {\n \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n \t\tce_k->ce_flags |= CE_HASHED;\n-\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n-\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n+\t\tif (!S_ISSPARSEDIR(ce_k->ce_mode)) {\n+\t\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n+\t\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n+\t\t}\n \t}\n \n \treturn NULL;\n\nbase-commit: 8613c2bb6cd16ef530dc5dd74d3b818a1ccbf1c0\n-- \ngitgitgadget\n"},{"id":"518621","messageId":"CAC97Eby+FQd3p5YVqgvvaYuxKD1yoUPWJ7RSuFNd4Gd92+ZSxg@mail.gmail.com","threadId":"63493","inReplyTo":"xmqqjz698xqb.fsf@gitster.g","subject":"Re: [PATCH v2] name-hash: don't add sparse directories in threaded lazy init","fromName":"Alex Mironov","fromEmail":"alexandrfox@gmail.com","sentAt":"2025-05-21T21:40:16Z","receivedAt":"2025-05-21T21:40:32Z","isPatch":true,"sender":{"key":"alexandrfox@gmail.com","avatar":"https://avatars.githubusercontent.com/u/199416?v=4"},"body":"Dear Junio,\n\nThank you for the feedback, I adjusted the commit message in full\nwhich hopefully makes this patch clearer. At the same time I don't\nquite understand the need for the perceived hostility - this is not\nsome \"fuzzy\" words, but the messaging from the original author of the\nsparse feature. I certainly understand your desire to uphold the\nstandards of contributions to git, especially from the new author, but\nI must say I feel quite alienated by your reply.\n\nNonetheless, the adjustment is submitted now and I hope that further\ncontributions remain welcomed.\n\nOn Wed, May 21, 2025 at 11:23 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > Alex Mironov writes:\n> >\n> >> Hey Junio,\n> >>\n> >> With respect to messaging I more or less copy-pasted Derricks message\n> >> from the original commit for non-threaded init: please check the\n> >> referenced commit. Let me know if another wording is needed/preferred.\n> >\n> > I know what you did. Copying and pasting others fuzzy words into\n> > your commit log message does not make your commit log message clear.\n> >\n> > I already said the given message is less clear than desired, so do I\n> > still have to let you know???\n>\n> Actually after re-reading what Derrick wrote in that commit, I\n> notice that you didn't even copy-pasted his message in full. Here\n> is the message in 5f116695 (name-hash: don't add directories to\n> name_hash, 2021-04-12):\n>\n> name-hash: don't add directories to name_hash\n>\n> Sparse directory entries represent a directory that is outside the\n> sparse-checkout definition. These are not paths to blobs, so should not\n> be added to the name_hash table. Instead, they should be added to the\n> directory hashtable when 'ignore_case' is true.\n>\n> Add a condition to avoid placing sparse directories into the name_hash\n> hashtable. This avoids filling the table with extra entries that will\n> never be queried.\n>\n> Notice that the second paragraph here makes it clear that how extra\n> entries would not contribute to or hurt the correctness? You failed\n> to copy-paste that crucial bit, which ended up making your version\n> of the explanation much less clear why the change would not affect\n> correctness than it could have been.\n\n\n\n--\nBest,\nAlex Mironov\n"},{"id":"518622","messageId":"xmqqa5758wlk.fsf@gitster.g","threadId":"63493","inReplyTo":"pull.1970.v3.git.git.1747862971672.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] name-hash: don't add sparse directories in threaded lazy init","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T21:47:35Z","receivedAt":"2025-05-21T21:47:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Mironov via GitGitGadget\" writes:\n\n> From: Alex Mironov \n>\n> Ensure that logic added in 5f11669586 (name-hash: don't add directories\n> to name_hash, 2021-04-12) also applies in multithreaded hashtable init\n> path.\n>\n> As per the original single-threaded change above: sparse directory entries\n> represent a directory that is outside the sparse-checkout definition.\n> These are not paths to blobs, so should not be added to the name_hash\n> table. Instead, they should be added to the directory hashtable when\n> 'ignore_case' is true.\n>\n> Add a condition to avoid placing sparse directories into the name_hash\n> hashtable. This avoids filling the table with extra entries that will\n> never be queried.\n>\n> Signed-off-by: Alex Mironov \n> ---\n\nSounds quite sensible and the above reads better.\n\n> diff --git a/name-hash.c b/name-hash.c\n> index d66de1cdfd5..b91e2762678 100644\n> --- a/name-hash.c\n> +++ b/name-hash.c\n> @@ -492,8 +492,10 @@ static void *lazy_name_thread_proc(void *_data)\n> \tfor (k = 0; k < d->istate->cache_nr; k++) {\n> \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n> \t\tce_k->ce_flags |= CE_HASHED;\n> -\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n> -\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n> +\t\tif (!S_ISSPARSEDIR(ce_k->ce_mode)) {\n> +\t\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n> +\t\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n> +\t\t}\n> \t}\n\n\nThis unfortunately gives us deeper nesting than your previous round,\nbut the conditional matches the original commit and easier to see\nwhat is going on by comparing the uni- and multi-threaded variants.\n\nNicely done. Will queue.\n\nThanks.\n\n\n"},{"id":"518635","messageId":"1ca297cc-35d8-4f8f-98d8-fc8d27cee2de@gmail.com","threadId":"63493","inReplyTo":"xmqqa5758wlk.fsf@gitster.g","subject":"Re: [PATCH v3] name-hash: don't add sparse directories in threaded lazy init","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-05-22T01:48:49Z","receivedAt":"2025-05-22T01:48:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/21/2025 5:47 PM, Junio C Hamano wrote:\n> \"Alex Mironov via GitGitGadget\" writes:\n> \n>> From: Alex Mironov \n>>\n>> Ensure that logic added in 5f11669586 (name-hash: don't add directories\n>> to name_hash, 2021-04-12) also applies in multithreaded hashtable init\n>> path.\n>>\n>> As per the original single-threaded change above: sparse directory entries\n>> represent a directory that is outside the sparse-checkout definition.\n>> These are not paths to blobs, so should not be added to the name_hash\n>> table. Instead, they should be added to the directory hashtable when\n>> 'ignore_case' is true.\n>>\n>> Add a condition to avoid placing sparse directories into the name_hash\n>> hashtable. This avoids filling the table with extra entries that will\n>> never be queried.\n>>\n>> Signed-off-by: Alex Mironov \n>> ---\n> \n> Sounds quite sensible and the above reads better.\n> \n>> diff --git a/name-hash.c b/name-hash.c\n>> index d66de1cdfd5..b91e2762678 100644\n>> --- a/name-hash.c\n>> +++ b/name-hash.c\n>> @@ -492,8 +492,10 @@ static void *lazy_name_thread_proc(void *_data)\n>> \tfor (k = 0; k < d->istate->cache_nr; k++) {\n>> \t\tstruct cache_entry *ce_k = d->istate->cache[k];\n>> \t\tce_k->ce_flags |= CE_HASHED;\n>> -\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n>> -\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n>> +\t\tif (!S_ISSPARSEDIR(ce_k->ce_mode)) {\n>> +\t\t\thashmap_entry_init(&ce_k->ent, d->lazy_entries[k].hash_name);\n>> +\t\t\thashmap_add(&d->istate->name_hash, &ce_k->ent);\n>> +\t\t}\n>> \t}\n> \n> \n> This unfortunately gives us deeper nesting than your previous round,\n> but the conditional matches the original commit and easier to see\n> what is going on by comparing the uni- and multi-threaded variants.\n> \n> Nicely done. Will queue.\nI agree that this version is ready to go. Thanks for shepherding\nthe v2 discussion.\n\n-Stolee\n"}]}