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

From: Paul Mackerras <paulus@samba.org>
Date: Oct 10, 2008, 11:48 UTC
Message-ID: <18671.16658.667581.499095@cargo.ozlabs.ibm.com>
In-Reply-To: <1222772422-28020-5-git-send-email-angavrilov@gmail.com>

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.

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?

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.

Paul.