Re: [PATCH v2] macos: lazily initialize iconv

["Torsten Bögershausen" <tboegi@web.de>]

On 10.08.12 20:18, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@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 <gitster@pobox.com>
>>> Helped-by: Ralf Thielow <ralf.thielow@gmail.com>
>>> Helped-by: Linus Torvalds <torvalds@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 <tboegi@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).
>
> Thanks.
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.

git v1.7.12.rc2:
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"
real    0m0.469s
user    0m0.283s
sys     0m0.175s

real    0m0.461s
user    0m0.283s
sys     0m0.170s

real    0m0.460s
user    0m0.283s
sys     0m0.170s

real    0m0.461s
user    0m0.283s
sys     0m0.177s

real    0m0.460s
user    0m0.283s
sys     0m0.176s

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"
real    0m0.463s
user    0m0.282s
sys     0m0.173s

real    0m0.460s
user    0m0.281s
sys     0m0.172s

real    0m0.460s
user    0m0.281s
sys     0m0.172s

real    0m0.463s
user    0m0.281s
sys     0m0.175s

real    0m0.462s
user    0m0.281s
sys     0m0.177s