Re: [PATCH v2] macos: lazily initialize iconv

From: Torsten Bögershausen <tboegi@web.de>
Date: Aug 10, 2012, 21:11 UTC
Message-ID: <50257906.1010606@web.de>
In-Reply-To: <7vsjbuk3v3.fsf@alter.siamese.dyndns.org>

On 10.08.12 20:18, Junio C Hamano wrote:

Show 30 quoted lines

> 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.463s user 0m0.281s sys 0m0.175s

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