[Kde-pim] Review Request 124967: Try to optimize KMime::CRLFtoLF() and KMime::LFtoCRLF() in the optimal cases when no conversion is needed

[Volker Krause]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124967/#review84538
-----------------------------------------------------------

Ship it!


Nice :)

Unfortunately I don't think the assumption holds though, these functions are used to process arbitrary crappy input from the internet, if there is one thing you can be sure about it's not following any rules ;-(

I think the real solution is to avoid those methods altogether in the long run and make sure our parsers can handle both line endings.

- Volker Krause


On Aug. 28, 2015, 11:51 a.m., Daniel Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124967/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 11:51 a.m.)
> 
> 
> Review request for KDEPIM and Volker Krause.
> 
> 
> Repository: kmime
> 
> 
> Description
> -------
> 
> `QByteArray::indexOf()` is way cheaper than `QByteArray::replace()` and does not cause detach. This hugely optimizes the ideal case when the data passed to `KMime::CRLFtoLF()` already use `\n` instead of `\r\n` (basically all mails written by KMime), because it does not trigger deep copy.
> 
> In the bad case when we actually need to do the replace the added cost of `indexOf()` is negligable compared to `replace()`, especially because `indexOf()` will return after scanning only single line of the data until running to the first `\r\n` (in most cases, anyway, and that's what we are optimizing for).
> 
> 
> We could optimize the `CRLFtoLF()` case even further by assuming that the entire document uses either `\n` or `\r\n` consistently. In that case we would be able to avoid scanning the entire document for `\r\n` but instead only look for the first `\n` and check if it's preceeded by `\r` or not. This basically means that in both cases (good and bad) we would only ever need to scan the first line of the document, and then return immediatelly or go for the `replace()`:
> 
> 
>     const int pos = s.indexOf('\n');
>     if (pos < 1 || s[pos - 1] != '\r') {
>         return s;
>     }
>     ...
> 
> 
> Question is whether the assumption about consistent use of line ends is acceptable.
> 
> 
> Diffs
> -----
> 
>   src/kmime_util.cpp 47ca087 
> 
> Diff: https://git.reviewboard.kde.org/r/124967/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/