Re: [PATCH] dir: force untracked cache with core.untrackedCache

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked
> cache more frequently than the core.untrackedCache config variable. This
> is due to how read_directory() handles the creation of an untracked
> cache. The old mechanism required using something like 'git update-index
> --untracked-cache' before the index would actually contain an untracked
> cache. This was noted as a performance problem on macOS in the past, and
> this is a resolution for that issue.

"The old mechanism" meaning "core.untrackedCache does not add a new
one; it only updates an existing one"?  What "this" refers to that
was noted as a problem on macOS is not quite clear; is "writing
untracked cache is a performance problem"? And the last "this" which
is a resolution is "not to add untrackedCache merely because the
configuration variable says we are allowed to use it"?

> The decision to not write the untracked cache without an environment
> variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty
> for changes to the untracked cache, 2018-02-05). The motivation of that
> change is that writing the index is expensive, and if the untracked
> cache is the only thing that needs to be written, then it is more
> expensive than the benefit of the cache. However, this also means that
> the untracked cache never gets populated, so the user who enabled it via
> config does not actually get the extension until running 'git
> update-index --untracked-cache' manually or using the environment
> variable.

OK.  It was invented solely as a test mechanism it seems, but at
least to the workflow of Microsoft folks, once we spent cycles to
prepare UNTR data, it helps their future use of the index to spend
a bit more cycle to write it out, instead of discarding.

I have to wonder if there are workflows that are sufficiently
different from what Microsoft folks use that the write-out cost of
more frequent updates to the untracked cache outweigh the runtime
performance boost of not having to run around and readdir() for
untracked files?

ad0fb659 (repo-settings: parse core.untrackedCache, 2019-08-13)
explains that unset core.untrackedCache means "keep", and "true"
means untracked cache is "automatically added", which this change is
not invalidated, so I guess there is no need to update anything in
the documentation for this change.  In fact, we might be able to
sell this change as a bugfix (i.e. "I set the configuration to
'true' but it wasn't written out when it should have").

> diff --git a/dir.c b/dir.c
> index d91295f2bcd..79a5f6918c8 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2936,7 +2936,9 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,
>  
>  		if (force_untracked_cache < 0)
>  			force_untracked_cache =
> -				git_env_bool("GIT_FORCE_UNTRACKED_CACHE", 0);
> +				git_env_bool("GIT_FORCE_UNTRACKED_CACHE", -1);
> +		if (force_untracked_cache < 0)
> +			force_untracked_cache = (istate->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE);
>  		if (force_untracked_cache &&
>  			dir->untracked == istate->untracked &&
>  		    (dir->untracked->dir_opened ||
>
> base-commit: b80121027d1247a0754b3cc46897fee75c050b44