Re: [PATCH (GITK) v3 0/4] Enhance encoding support.

From: Paul Mackerras <paulus@samba.org>
Date: Oct 15, 2008, 12:32 UTC
Message-ID: <18677.58054.209169.479165@cargo.ozlabs.ibm.com>
In-Reply-To: <1223885554-27718-1-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

Show 5 quoted lines

> Currently GUI tools don't provide enough support for
> viewing files that contain non-ASCII characters. This set of
> patches addresses this issue. The git-gui part of the series
> is based on patches that were in the 'pu' branch of the
> git-gui repository.

Thanks. I applied 1/4 and 2/4 as one commit and 3/4 as another. However, it seems invoking git check-attr is quite slow, or at least, there is a large startup cost which is not really amortized by batching up only 30 paths per call.

As a test I tried displaying the diff in the kernel tree from 2.6.27 to Linus' master as of a day or so ago (3fa8749e) which has 5277 files different. With the 30-way batching this takes over 6 seconds to get the file encodings (all of which are "unspecified" since I don't have any .gitattributes files), which we need before we display the list of changed files, which before only took 0.12 seconds -- so we're 50x slower at showing the list of changed files as a result of adding the per-file encoding support. Also, each call to cache_gitattr was taking 1.2 seconds, which is too long since file handlers are supposed to limit themselves to less than 100ms (preferably less than 50ms) of processing on any one call, and the cache_gitattr call was making gettreediffline take way too long.

So I have added another commit which makes the per-file encoding support optional and default to off for now. It also increases the amount of batching in cache_gitattr (except under windows) and reduces the number of lines that gettreediffline does when per-file encoding support is on. Along the way, it also converts the new code to the gitk coding style, cleans up a few things and adds a simple cache to tclencoding.

If git check-attr gets the --stdin-paths flag added, that will be good, except then getblobdiffline will need to do something clever if it needs the encoding for a file but the result hasn't come back from git-check-attr yet. That could get quite tricky in fact...

Paul.