{"thread":{"id":"57412","subject":"[PATCH] dir: force untracked cache with core.untrackedCache","startedAt":"2022-02-14T17:37:09Z","lastAt":"2022-02-17T22:51:35Z","messageCount":5,"participants":["Derrick Stolee via GitGitGadget","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"448373","messageId":"pull.1058.git.1644860224151.gitgitgadget@gmail.com","threadId":"57412","inReplyTo":null,"subject":"[PATCH] dir: force untracked cache with core.untrackedCache","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-14T17:37:04Z","receivedAt":"2022-02-14T17:37:09Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nThe GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\ncache more frequently than the core.untrackedCache config variable. This\nis due to how read_directory() handles the creation of an untracked\ncache. The old mechanism required using something like 'git update-index\n--untracked-cache' before the index would actually contain an untracked\ncache. This was noted as a performance problem on macOS in the past, and\nthis is a resolution for that issue.\n\nThe decision to not write the untracked cache without an environment\nvariable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty\nfor changes to the untracked cache, 2018-02-05). The motivation of that\nchange is that writing the index is expensive, and if the untracked\ncache is the only thing that needs to be written, then it is more\nexpensive than the benefit of the cache. However, this also means that\nthe untracked cache never gets populated, so the user who enabled it via\nconfig does not actually get the extension until running 'git\nupdate-index --untracked-cache' manually or using the environment\nvariable.\n\nWe have had a version of this change in the microsoft/git fork for a few\nmajor releases now. It has been working well to get users into a good\nstate. Yes, that first index write is slow, but the remaining index\nwrites are much faster than they would be without this change.\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n    dir: force untracked cache with core.untrackedCache\n    \n    We have seen users in the wild that have had core.untrackedCache\n    enabled, but never actually have an untracked cache created for them. We\n    have a test in t7063 that shows git status should write the untracked\n    cache, so I'm not exactly sure how users are in this state for long.\n    \n    This patch fixes the situation. I also know of another group that sets\n    GIT_FORCE_UNTRACKED_CACHE=1 in their developer environment in order to\n    get this behavior.\n    \n    -Stolee\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1058%2Fderrickstolee%2Funtracked-cache-write-more-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1058/derrickstolee/untracked-cache-write-more-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1058\n\n dir.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex d91295f2bcd..79a5f6918c8 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2936,7 +2936,9 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,\n \n \t\tif (force_untracked_cache < 0)\n \t\t\tforce_untracked_cache =\n-\t\t\t\tgit_env_bool(\"GIT_FORCE_UNTRACKED_CACHE\", 0);\n+\t\t\t\tgit_env_bool(\"GIT_FORCE_UNTRACKED_CACHE\", -1);\n+\t\tif (force_untracked_cache < 0)\n+\t\t\tforce_untracked_cache = (istate->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE);\n \t\tif (force_untracked_cache &&\n \t\t\tdir->untracked == istate->untracked &&\n \t\t    (dir->untracked->dir_opened ||\n\nbase-commit: b80121027d1247a0754b3cc46897fee75c050b44\n-- \ngitgitgadget\n"},{"id":"448385","messageId":"3dd526a6-9160-97cd-05a3-769f5871587d@github.com","threadId":"57412","inReplyTo":"xmqqzgmt19w6.fsf@gitster.g","subject":"Re: [PATCH] dir: force untracked cache with core.untrackedCache","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-02-14T20:40:26Z","receivedAt":"2022-02-14T20:45:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/14/2022 3:16 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>> From: Derrick Stolee <derrickstolee@github.com>\n>>\n>> The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\n>> cache more frequently than the core.untrackedCache config variable. This\n>> is due to how read_directory() handles the creation of an untracked\n>> cache. The old mechanism required using something like 'git update-index\n>> --untracked-cache' before the index would actually contain an untracked\n>> cache. This was noted as a performance problem on macOS in the past, and\n>> this is a resolution for that issue.\n> \n> \"The old mechanism\" meaning \"core.untrackedCache does not add a new\n> one; it only updates an existing one\"?  What \"this\" refers to that\n> was noted as a problem on macOS is not quite clear; is \"writing\n> untracked cache is a performance problem\"? And the last \"this\" which\n> is a resolution is \"not to add untrackedCache merely because the\n> configuration variable says we are allowed to use it\"?\n\nRight. I can see how that is confusing. Here's another attempt:\n\n   The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\n   cache more frequently than the core.untrackedCache config variable. This\n   is due to how read_directory() handles the creation of an untracked\n   cache.\n\n   Before this change, Git would not create the untracked cache extension\n   for an index that did not already have one. Users would need to run a\n   command such as 'git update-index --untracked-cache' before the index\n   would actually contain an untracked cache.\n\n   In particular, users noticed that the untracked cache would not appear\n   even with core.untrackedCache=true. Some users reported setting\n   GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to\n   ensure the untracked cache would be created.\n \n>> The decision to not write the untracked cache without an environment\n>> variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty\n>> for changes to the untracked cache, 2018-02-05). The motivation of that\n>> change is that writing the index is expensive, and if the untracked\n>> cache is the only thing that needs to be written, then it is more\n>> expensive than the benefit of the cache. However, this also means that\n>> the untracked cache never gets populated, so the user who enabled it via\n>> config does not actually get the extension until running 'git\n>> update-index --untracked-cache' manually or using the environment\n>> variable.\n> \n> OK.  It was invented solely as a test mechanism it seems, but at\n> least to the workflow of Microsoft folks, once we spent cycles to\n> prepare UNTR data, it helps their future use of the index to spend\n> a bit more cycle to write it out, instead of discarding.\n> \n> I have to wonder if there are workflows that are sufficiently\n> different from what Microsoft folks use that the write-out cost of\n> more frequent updates to the untracked cache outweigh the runtime\n> performance boost of not having to run around and readdir() for\n> untracked files?\n\nI think the only difference here is the transition state from no cache\nto an existing cache. From then on, the cache is kept up-to-date with\nthe same frequency as without this change.\n\n> ad0fb659 (repo-settings: parse core.untrackedCache, 2019-08-13)\n> explains that unset core.untrackedCache means \"keep\", and \"true\"\n> means untracked cache is \"automatically added\", which this change is\n> not invalidated, so I guess there is no need to update anything in\n> the documentation for this change.  In fact, we might be able to\n> sell this change as a bugfix (i.e. \"I set the configuration to\n> 'true' but it wasn't written out when it should have\").\n\nYes, I believe this to be the case. Hopefully the rewritten\nparagraphs above make this more clear.\n\nThanks,\n-Stolee\n"},{"id":"448388","messageId":"xmqqzgmt19w6.fsf@gitster.g","threadId":"57412","inReplyTo":"pull.1058.git.1644860224151.gitgitgadget@gmail.com","subject":"Re: [PATCH] dir: force untracked cache with core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-14T20:16:41Z","receivedAt":"2022-02-14T20:57:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Derrick Stolee <derrickstolee@github.com>\n>\n> The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\n> cache more frequently than the core.untrackedCache config variable. This\n> is due to how read_directory() handles the creation of an untracked\n> cache. The old mechanism required using something like 'git update-index\n> --untracked-cache' before the index would actually contain an untracked\n> cache. This was noted as a performance problem on macOS in the past, and\n> this is a resolution for that issue.\n\n\"The old mechanism\" meaning \"core.untrackedCache does not add a new\none; it only updates an existing one\"?  What \"this\" refers to that\nwas noted as a problem on macOS is not quite clear; is \"writing\nuntracked cache is a performance problem\"? And the last \"this\" which\nis a resolution is \"not to add untrackedCache merely because the\nconfiguration variable says we are allowed to use it\"?\n\n> The decision to not write the untracked cache without an environment\n> variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty\n> for changes to the untracked cache, 2018-02-05). The motivation of that\n> change is that writing the index is expensive, and if the untracked\n> cache is the only thing that needs to be written, then it is more\n> expensive than the benefit of the cache. However, this also means that\n> the untracked cache never gets populated, so the user who enabled it via\n> config does not actually get the extension until running 'git\n> update-index --untracked-cache' manually or using the environment\n> variable.\n\nOK.  It was invented solely as a test mechanism it seems, but at\nleast to the workflow of Microsoft folks, once we spent cycles to\nprepare UNTR data, it helps their future use of the index to spend\na bit more cycle to write it out, instead of discarding.\n\nI have to wonder if there are workflows that are sufficiently\ndifferent from what Microsoft folks use that the write-out cost of\nmore frequent updates to the untracked cache outweigh the runtime\nperformance boost of not having to run around and readdir() for\nuntracked files?\n\nad0fb659 (repo-settings: parse core.untrackedCache, 2019-08-13)\nexplains that unset core.untrackedCache means \"keep\", and \"true\"\nmeans untracked cache is \"automatically added\", which this change is\nnot invalidated, so I guess there is no need to update anything in\nthe documentation for this change.  In fact, we might be able to\nsell this change as a bugfix (i.e. \"I set the configuration to\n'true' but it wasn't written out when it should have\").\n\n> diff --git a/dir.c b/dir.c\n> index d91295f2bcd..79a5f6918c8 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -2936,7 +2936,9 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,\n>  \n>  \t\tif (force_untracked_cache < 0)\n>  \t\t\tforce_untracked_cache =\n> -\t\t\t\tgit_env_bool(\"GIT_FORCE_UNTRACKED_CACHE\", 0);\n> +\t\t\t\tgit_env_bool(\"GIT_FORCE_UNTRACKED_CACHE\", -1);\n> +\t\tif (force_untracked_cache < 0)\n> +\t\t\tforce_untracked_cache = (istate->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE);\n>  \t\tif (force_untracked_cache &&\n>  \t\t\tdir->untracked == istate->untracked &&\n>  \t\t    (dir->untracked->dir_opened ||\n>\n> base-commit: b80121027d1247a0754b3cc46897fee75c050b44\n"},{"id":"448734","messageId":"pull.1058.v2.git.1645131630140.gitgitgadget@gmail.com","threadId":"57412","inReplyTo":"pull.1058.git.1644860224151.gitgitgadget@gmail.com","subject":"[PATCH v2] dir: force untracked cache with core.untrackedCache","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-02-17T21:00:29Z","receivedAt":"2022-02-17T21:00:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <derrickstolee@github.com>\n\nThe GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\ncache more frequently than the core.untrackedCache config variable. This\nis due to how read_directory() handles the creation of an untracked\ncache.\n\nBefore this change, Git would not create the untracked cache extension\nfor an index that did not already have one. Users would need to run a\ncommand such as 'git update-index --untracked-cache' before the index\nwould actually contain an untracked cache.\n\nIn particular, users noticed that the untracked cache would not appear\neven with core.untrackedCache=true. Some users reported setting\nGIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to\nensure the untracked cache would be created.\n\nThe decision to not write the untracked cache without an environment\nvariable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty\nfor changes to the untracked cache, 2018-02-05). The motivation of that\nchange is that writing the index is expensive, and if the untracked\ncache is the only thing that needs to be written, then it is more\nexpensive than the benefit of the cache. However, this also means that\nthe untracked cache never gets populated, so the user who enabled it via\nconfig does not actually get the extension until running 'git\nupdate-index --untracked-cache' manually or using the environment\nvariable.\n\nWe have had a version of this change in the microsoft/git fork for a few\nmajor releases now. It has been working well to get users into a good\nstate. Yes, that first index write is slow, but the remaining index\nwrites are much faster than they would be without this change.\n\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n    dir: force untracked cache with core.untrackedCache\n    \n    We have seen users in the wild that have had core.untrackedCache\n    enabled, but never actually have an untracked cache created for them. We\n    have a test in t7063 that shows git status should write the untracked\n    cache, so I'm not exactly sure how users are in this state for long.\n    \n    This patch fixes the situation. I also know of another group that sets\n    GIT_FORCE_UNTRACKED_CACHE=1 in their developer environment in order to\n    get this behavior.\n    \n    -Stolee\n    \n    \n    Update in v2\n    ============\n    \n     * Edited the commit message to be clearer.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1058%2Fderrickstolee%2Funtracked-cache-write-more-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1058/derrickstolee/untracked-cache-write-more-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1058\n\nRange-diff vs v1:\n\n 1:  061ee9a379d ! 1:  8d132bc5566 dir: force untracked cache with core.untrackedCache\n     @@ Commit message\n          The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\n          cache more frequently than the core.untrackedCache config variable. This\n          is due to how read_directory() handles the creation of an untracked\n     -    cache. The old mechanism required using something like 'git update-index\n     -    --untracked-cache' before the index would actually contain an untracked\n     -    cache. This was noted as a performance problem on macOS in the past, and\n     -    this is a resolution for that issue.\n     +    cache.\n     +\n     +    Before this change, Git would not create the untracked cache extension\n     +    for an index that did not already have one. Users would need to run a\n     +    command such as 'git update-index --untracked-cache' before the index\n     +    would actually contain an untracked cache.\n     +\n     +    In particular, users noticed that the untracked cache would not appear\n     +    even with core.untrackedCache=true. Some users reported setting\n     +    GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to\n     +    ensure the untracked cache would be created.\n      \n          The decision to not write the untracked cache without an environment\n          variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty\n\n\n dir.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex d91295f2bcd..79a5f6918c8 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2936,7 +2936,9 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,\n \n \t\tif (force_untracked_cache < 0)\n \t\t\tforce_untracked_cache =\n-\t\t\t\tgit_env_bool(\"GIT_FORCE_UNTRACKED_CACHE\", 0);\n+\t\t\t\tgit_env_bool(\"GIT_FORCE_UNTRACKED_CACHE\", -1);\n+\t\tif (force_untracked_cache < 0)\n+\t\t\tforce_untracked_cache = (istate->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE);\n \t\tif (force_untracked_cache &&\n \t\t\tdir->untracked == istate->untracked &&\n \t\t    (dir->untracked->dir_opened ||\n\nbase-commit: b80121027d1247a0754b3cc46897fee75c050b44\n-- \ngitgitgadget\n"},{"id":"448743","messageId":"xmqq5ypd6r9s.fsf@gitster.g","threadId":"57412","inReplyTo":"pull.1058.v2.git.1645131630140.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] dir: force untracked cache with core.untrackedCache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-17T22:51:27Z","receivedAt":"2022-02-17T22:51:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Derrick Stolee <derrickstolee@github.com>\n>\n> The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked\n> cache more frequently than the core.untrackedCache config variable. This\n> is due to how read_directory() handles the creation of an untracked\n> cache.\n>\n> Before this change, Git would not create the untracked cache extension\n> for an index that did not already have one. Users would need to run a\n> command such as 'git update-index --untracked-cache' before the index\n> would actually contain an untracked cache.\n>\n> In particular, users noticed that the untracked cache would not appear\n> even with core.untrackedCache=true. Some users reported setting\n> GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to\n> ensure the untracked cache would be created.\n>\n> The decision to not write the untracked cache without an environment\n> variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty\n> for changes to the untracked cache, 2018-02-05). The motivation of that\n> change is that writing the index is expensive, and if the untracked\n> cache is the only thing that needs to be written, then it is more\n> expensive than the benefit of the cache. However, this also means that\n> the untracked cache never gets populated, so the user who enabled it via\n> config does not actually get the extension until running 'git\n> update-index --untracked-cache' manually or using the environment\n> variable.\n>\n> We have had a version of this change in the microsoft/git fork for a few\n> major releases now. It has been working well to get users into a good\n> state. Yes, that first index write is slow, but the remaining index\n> writes are much faster than they would be without this change.\n>\n> Signed-off-by: Derrick Stolee <derrickstolee@github.com>\n> ---\n\nWill queue.\n\nThanks.\n"}]}