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

[Paul Mackerras <paulus@samba.org>]

Alexander Gavrilov writes:

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

Eeek.  OK.

> > 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.

I was thinking more in terms of dropping the path_attr_cache actually.

Actually, if [tcl_encoding] is slow, then why is $gui_encoding the
untranslated version, so that we do [tcl_encoding $gui_encoding] on
each call to get_path_encoding?  Why don't we do the tcl_encoding call
once and have $gui_encoding be the result of that?  In fact
$gui_encoding should be the result of this code (from
get_path_encoding):

	set tcl_enc [tcl_encoding $gui_encoding]
	if {$tcl_enc eq {}} {
		set tcl_enc [encoding system]
	}

And if [tcl_encoding] is slow, then it should have a cache.  There's
only likely to be at most 2 or 3 values it gets called for, and it's
a constant function.

At this point, what I think I might do is apply your set of patches
(but with 2/4 and 3/4 folded into a single patch) and then go through
and do another commit that addresses the concerns I've raised.  OK?

Paul.