Am Freitag, 27. Oktober 2006 17:03 schrieb Abdelrazak Younes:
> Hello,
> 
> This patch brings a 20% improvement when loading a big document (my test 
> document is the UserGuide copied&Pasted 3 times). This is done by 
> avoiding multiple string/vector/docstring copying.
> 
> I can think of many other optimisation but I would like this one to go 
> in first. IMHO, most of unicode can go and only one iconv_convert() 
> should stay.
> 
> Objections?

You will probably get some from Lars, but I have one of my own: I don't 
like the replacement of iconv_t by int and the resulting casting. Please 
revert that. The casting of -1 to iconv_t is safer. Do you really know 
that int will always be compatible to iconv_t?

IMO an important bit is missing: Completing a conversion that is incomplete 
because the buffer was too small. We cannot always increase the bufffer if 
that happens, so IMO we need to do that for 1.5.0, because users will 
otherwise get silent data corruption.

With that in mind I do not like that you make iconv_convert a part of the 
public interface. It has an ugly interface that should be kept private. It 
should be possible to do an efficient but still clean design IMHO.


Georg

Reply via email to