FastSaxSerializer::write ...

[Chris Sherlock]

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 at 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 at collabora.com <mailto:michael.meeks at collabora.com>>:
> Hi Mark,
> 
>         Great to see:
> 
> commit e99f22bbc499ab0566621ee0bb01e4a7747efe76
> Author: Mark Hung <marklh9 at gmail.com <mailto:marklh9 at 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 at collabora.com <mailto:michael.meeks at collabora.com>  <><, Pseudo Engineer, itinerant idiot
> 
> 
> 
> 
> -- 
> Mark Hung
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org <mailto:LibreOffice at lists.freedesktop.org>
> http://lists.freedesktop.org/mailman/listinfo/libreoffice <http://lists.freedesktop.org/mailman/listinfo/libreoffice>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20160118/01542563/attachment.html>