On Tue, Apr 22, 2014 at 4:56 PM, Karsten Blees <karsten.bl...@gmail.com> wrote:
> Am 17.04.2014 07:51, schrieb Nguyễn Thái Ngọc Duy:
>> This patch serves as a heads up about a feature I'm working on. I hope
>> that by posting it early, people could double check if I have made
>> some fundamental mistakes that completely ruin the idea. It's about
>> speeding up "git status" by caching untracked file info in the index
>> _if_ your file system supports it (more below).
>> The whole WIP series is at
>> https://github.com/pclouds/git/commits/untracked-cache
>> I only post the real meat here. I'm aware of a few incomplete details
>> in this patch, but nothing fundamentally wrong. So far the numbers are
>> promising.  ls-files is updated to run fill_directory() twice in a
>> row and "ls-files -o --directory --no-empty-directory --exclude-standard"
>> (with gcc -O0) gives me:
>>            first run  second (cached) run
>> gentoo-x86    500 ms             71.6  ms
>> wine          140 ms              9.72 ms
>> webkit        125 ms              6.88 ms
> IIRC name_hash.c::lazy_init_name_hash took ~100ms on my system, so hopefully 
> you did a dummy 'cache_name_exists("anything")' before starting the 
> measurement of the first run?

No I didn't. Thanks for pointing it out. I'll see if I can reduce its time.

>> The following inputs are sufficient to determine what files in a
>> directory are excluded:
>>  - The list of files and directories of the direction in question
>>  - The $GIT_DIR/index
>>  - The content of $GIT_DIR/info/exclude
>>  - The content of core.excludesfile
>>  - The content (or the lack) of .gitignore of all parent directories
>>    from $GIT_WORK_TREE
> The dir_struct.flags also play a big role in evaluation of read_directory.
> E.g. it seems untracked files are not properly recorded if the cache is 
> filled with '--ignored' option:

Yeah. dir_struct.flags will be part of the input. I intend to optimize
"git status" case only, so if it matches the recorded
dir_struct.flags, the cache is used. Else the cache is ignored.
Caching --ignored is not so interesting, because the list of ignored
files could be huge, while untracked listing is usually small.

>> @@ -1360,15 +1603,18 @@ static enum path_treatment 
>> read_directory_recursive(struct dir_struct *dir,
>>                       break;
>>               case path_untracked:
>> -                     if (!(dir->flags & DIR_SHOW_IGNORED))
>> -                             dir_add_name(dir, path.buf, path.len);
>> +                     if (dir->flags & DIR_SHOW_IGNORED)
>> +                             break;
>> +                     dir_add_name(dir, path.buf, path.len);
>> +                     if (cdir.fdir)
>> +                             add_untracked(untracked, path.buf + baselen);
>>                       break;
> Similarly, the '--directory' option controls early returns from the directory 
> scan (via read_directory_recursive's check_only argument), so you won't be 
> able to get a full untracked files listing if the cache was recorded with 
> '--directory'. Additionally, '--directory' aggregates the state at the 
> topmost untracked directory, so that directory's cached state depends on all 
> sub-directories as well...

I missed this. We could ignore check_only if caching is enabled, but
that does not sound really good. Let me think about it more..

> I wonder if it makes sense to separate cache recording logic from 
> read_directory_recursive and friends, which are mainly concerned with flags 
> processing.

The core code path is still shared though, or we would duplicate r_d_r
entirely for caching recording, which sounds like a maintenance

>> At the implementation level, the whole directory structure is saved,
>> each directory corresponds to one struct untracked_dir.
> With the usual options (e.g. standard 'git status'), untracked directories 
> are mostly skipped, so the cache would mostly store tracked directories. 
> Naming it 'struct untracked_dir' is a bit confusing, IMO.

It's actually just "directories". We may need to store both tracked
and untracked directories. Maybe renaming it to cached_dir..

>> So if all is really well, read_directory() becomes a series of
>> open(".gitignore"), read(".gitignore"), close(), hash_sha1_file() and
>> stat(<dir>) _without_ heavyweight exclude filtering. There should be
>> no overhead if this feature is disabled.
> Wouldn't mtime of .gitignore files suffice here (so you don't need to open 
> and parse them every time)?

That's a further optimization. With the current code it's simpler to
open .gitignore. Assume you have a path a/b/c. a/.gitignore's stat
info is good, so you skip opening it. Then you find a/b/.gitignore is
modified and you need to recompute untracked files in a/b. To do that
you need a/.gitignore as well. Lazily opening a/.gitignore at this
stage is possible, but trickier (you have to make sure the rules are
in correct order because of negative patterns).

Anyway, the number of .gitignore files is usually small. We can
already avoid opening non-existent .gitignore, which is proportional
to the number of directories.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to