Re: [PATCH] read_directory: avoid invoking exclude machinery on tracked files

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 15, 2013, 18:30 UTC
Message-ID: <CACsJy8A6oBjbaX=3iQcSxcwed28KLTk_tN+iuWDLsC512Z2V1Q@mail.gmail.com>
In-Reply-To: <7vd2w1wmdo.fsf@alter.siamese.dyndns.org>

On Fri, Feb 15, 2013 at 11:52 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

> In the current code, we always check if a path is excluded, and when
> dealing with DT_REG/DT_LNK, we call treat_file():
>
>  * When such a path is excluded, treat_file() returns true when we
>    are not showing ignored directories. This causes treat_one_path()
>    to return path_ignored, so for excluded DT_REG/DT_LNK paths when
>    no DIR_*_IGNORED is in effect, this change is a correct
>    optimization.
>
>  * When such a path is not excluded, on the ther hand, and when we
>    are not showing ignored directories, treat_file() just returns
>    the value of exclude_file, which is initialized to false and is
>    not changed in the function.  This causes treat_one_path() to
>    return path_handled.  However, the new code returns path_ignored
>    in this case.
>
> What guarantees that this change is regression free?

If you consider read_directory_recursive alone, there is a regression. The return value of r_d_r depends on path_handled/path_ignored. With this patch, the return value will be different. The return value is only used by treat_directory() in two cases:

 - when DIR_SHOW_IGNORED is set, which disables the optimization so no
regression

 - when DIR_HIDE_EMPTY_DIRECTORIES is _not_ set (and neither is
DIR_SHOW_IGNORED), the optimization is still on and different r_d_r's
return value would lead to different behavior. However I don't think
it can happen.

treat_directory checks if the given directory can be found in index. In that case the neither r_d_r calls in treat_directory is reachable. If the given directory cannot be found in the index, the second r_d_r is reachable. But then the cache_name_exists() in the patch should always be false (parent not in index, children cannot), so the optimization is off and r_d_r returns correctly.

It's a bit tricky. I'm not sure if I miss anything else.

Show 7 quoted lines

> I do not seem
> to be able to find anything that checks if the path is already known
> to the index in the original code for the case you special cased
> (i.e. DIR_*_IGNORED is not set and dtype is not DT_DIR).  Do all the
> callers that reach this function in their callgraph, when they get
> path_ignored for a path in the index, behave as if the difference
> between path_ignored and path_handled does not matter?

-- 
Duy