On 10.08.12 20:18, Junio C Hamano wrote:
> Torsten Bögershausen <tbo...@web.de> writes:
>> On 31.07.12 20:37, Junio C Hamano wrote:
>>> In practice, the majority of paths do not have utf8 that needs
>>> the canonicalization. Lazily call iconv_open()/iconv_close() to
>>> avoid unnecessary overhead.
>>> Signed-off-by: Junio C Hamano <gits...@pobox.com>
>>> Helped-by: Ralf Thielow <ralf.thie...@gmail.com>
>>> Helped-by: Linus Torvalds <torva...@linux-foundation.org>
>>> * This is not even compile tested, so it needs testing and
>>> benchmarking, as I do not even know how costly the calls to
>>> open/close are when we do not have to call iconv() itself.
>> Hi Junio,
>> thanks for the optimization.
>> Tested-by: Torsten Bögershausen <tbo...@web.de>
> Well, I didn't mean the correctness testing without numbers. The
> correctness of the patch after a couple of people eyeballed it was
> no longer a question.
> If the patch does not give any measuable performance difference to
> people who exercise this codepath, it is not worth merging. And
> that is not something I can't do myself without a Mac (nor I wish to
> have one to be able to do so myself).
Really sorry for the confusion with the percentage, I should have written:
The patch is tested OK, and we can even remove 2 lines of code,
where we save and restore errno.
They are no longer needed.
Here some benchmarks on my system
(Intel Core 2 Duo @ 3,06 GHz, L2-Cache: 3 MB, Memory: 4 GB)
running git status on the linux source tree.
After cloning, I set "git config core.precomposeunicode true".
git is v1.7.12.rc2,
git_git/git is with your patch.
tb@birne:~/projects/git/linux-2.6> for f in 1 2 3 4 5; do time git status ;
done 2>&1 | egrep "^user|^real|^sys"
With lazy init of iconv:
tb@birne:~/projects/git/linux-2.6> for f in 1 2 3 4 5; do time
~/projects/git/git_git/git status ; done 2>&1 | egrep "^user|^real|^sys"
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