Re: FastSaxSerializer::write ...

I think a unit test might be helpful. They are actually quite easy to write - in fact, I wrote a 
very simple one the other day. 

Have a look on master at vcl/qa/cppunit/font.cxx

I’m trying to write a unit test for every low level component I change or refactor, I’ve found that 
it doesn’t take much time to do and I’m trying to do it if only to ensure that any code I modify 
doesn’t break unexpectedly when I make changes further down the line. 

Let me know if you get stuck :-)

Chris

> On 17 Jan 2016, at 12:36 AM, Mark Hung <marklh9@gmail.com> wrote:
>
> Hi Michael,
>
> I'd look into performance issue. 
> Is there any benchmark or unit test that I can use to check performance enhancement? 
> And what do you mean by regression test for the characters?
>
>
> 2016-01-16 19:33 GMT+08:00 Michael Meeks <michael.meeks@collabora.com 
> <mailto:michael.meeks@collabora.com>>:
> Hi Mark,
>
>         Great to see:
>
> commit e99f22bbc499ab0566621ee0bb01e4a7747efe76
> Author: Mark Hung <marklh9@gmail.com <mailto:marklh9@gmail.com>>
> Date:   Sun Jan 10 00:28:14 2016 +0800
>
>     Fix FastSaxSerializer::write() for non-BMP unicode characters.
>
>         Clearly we don't want to mangle UTF-16 etc. characters - and the code
> looks dodgy indeed for that =)
>
>         I'm somewhat suspicious though that this will cause some unhelpful
> performance regression; and I was wondering if you could look into
> fixing that ? and (ideally) helping out with a regression test for these
> characters ? =)
>
>         Memory allocation of OStrings etc. via rtl/sal is rather expensive in
> most of the profiles I've seen; and particularly doing it for every
> attribute ;-)
>
>         Would it be possible to replace the original code - but - as soon as we
> hit a non-ASCII character to convert the rest of the string, and write
> that - so something like:
>
>         write (OString(sOutput[i], nLength-i, RTL_TEXTENCODING_UTF8), bEscape);
>         return;
>
>         So we can get the character conversion and pairing correct - while
> keeping the lack of allocation there ? =)
>
>         Anyhow - thanks muchly for the fix ! great to see complex text support
> improve.
>
>         All the best,
>
>                 Michael.
>
> --
>  michael.meeks@collabora.com <mailto:michael.meeks@collabora.com>  <><, Pseudo Engineer, 
> itinerant idiot
>
>
>
>
> -- 
> Mark Hung
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org <mailto:LibreOffice@lists.freedesktop.org>
> http://lists.freedesktop.org/mailman/listinfo/libreoffice 
> <http://lists.freedesktop.org/mailman/listinfo/libreoffice>