Re: [PATCH (GITK) v2 4/4] gitk: Implement batch lookup and caching of encoding attrs.

["Alexander Gavrilov" <angavrilov@gmail.com>]

On Fri, Oct 10, 2008 at 3:48 PM, Paul Mackerras <paulus@samba.org> wrote:
> Alexander Gavrilov writes:
>
>> When the diff contains thousands of files, calling git-check-attr
>> once per file is very slow. With this patch gitk does attribute
>> lookup in batches of 30 files while reading the diff file list,
>> which leads to a very noticeable speedup.
>
> Why only 30 at a time?  The logic would be simpler if cache_gitattr
> just did all the paths in one call to git check-attr, and it should be
> able to cope with 1000 paths in one call, I would think, which is the
> most that gettreediffline will give it.

OS-enforced command-line size limit on Windows is 32K. Cramming in
1000 paths would leave only 32 characters for each path.

> Also, I wonder why we now have two levels of caching of the encoding
> attribute.  Your patch 1/4 introduced path_encoding_cache, which was
> fine, but now we have path_attr_cache as well, which seems to me to
> serve exactly the same function since the encoding is the only
> attribute we ever ask about.  Surely we don't need both caches?

If the (git-gui) patch that reimplements the tcl_encoding procedure is
applied, we may drop the path_encoding_cache. Current implementation
is too slow for batch lookup, especially if the encoding is actually
not supported, and without the cache the lookup would be done on every
loading of a diff.

> Even with this batching, I am a bit concerned that adding the encoding
> support might make things noticeably slower for people who don't need
> any encoding support (which would be the majority, I think).  We may
> end up needing an option to turn off the checking of the encoding
> attribute.

I hope that most diffs don't contain thousands of files at once. And
actual huge diffs are likely to be relatively slow to load anyway.

Alexander