Currently, Qt text codec takes the whole input string and decode it into unicode QString, and QString is then converted to WebCore::String for further processing. QString usually takes twice as much memory as the input string, and copying QString to WebCore::String needs the same amount memory. As a result, the peak memory consumption of decoding is 5 times as the input string (1x for original input + 2x for QString + 2x for WebCore::String). That means, for 1MB input text, at least 5 MB RAM is needed to decode it into unicode.
Created attachment 38868 [details] Cut big input buffer to small buffers to reduce peak memory during decoding. The fix cuts big input buffer to small buffers and feed them to the decoder; the decoded result is appended into the output buffer. This way, we can save ~40% of memory consumption. (5x of input buffer to 3x input buffer).
This looks OK to me, but Darin Adler is probably the best person to review this (small) change.
Comment on attachment 38868 [details] Cut big input buffer to small buffers to reduce peak memory during decoding. > + // We chop input buffer to smaller buffers to avoid excessive memory consumption > + // when the input buffer is big. This helps reduce peak memory consumption in > + // mobile devices where system RAM is limited. > + static const int kInputChunkSize = 32 * 1024; In Qt we don't use Hungarian notation :) You can safely drop "k" prefix there. > + const char* buf = bytes; > + const char* end = buf + length; > + String unicode; > + > + while (buf < end) { > + int size = end - buf; > + size = qMin(size, kInputChunkSize); > + QString decoded = m_codec->toUnicode(buf, size, &m_state); > + unicode.append(decoded); > + buf += size; > + } While this reduced the peak heap, my concern is the speed. Because we append a small block every time, String needs to grow and reallocate. Also QTextCode constantly allocates and deallocates 32KB QString. Can you use Valgrind to see the difference in the memory consumption? Can we perhaps specify the inputChunkSize, say a small value for Symbian but a larger one for other platforms? r- until these issues are addressed.
Created attachment 39652 [details] modified patch as per Ariya's comment. thanks for the comment, Ariya. I did memory check with valgrind, the peak memory saving is about 300KB for news.google.com. I modified the patch to use a small chunk size (32K) for Symbian while set it to 1MB for other platforms.
Created attachment 39653 [details] Oops! the previous patch has unnecessary changes to ChangeLog file :)
(In reply to comment #4) > Created an attachment (id=39652) [details] > modified patch as per Ariya's comment. > > thanks for the comment, Ariya. > > I did memory check with valgrind, the peak memory saving is about 300KB for > news.google.com. I modified the patch to use a small chunk size (32K) for > Symbian while set it to 1MB for other platforms. I guess a small chunk size is important for most embedded devices, so maybe the test would make sense for ARM in general?
I personally would find this useful for other devices that are not Symbian based, like one we're currently using for another Nokia product.
Committed r48752: <http://trac.webkit.org/changeset/48752>
Comment on attachment 39653 [details] Oops! the previous patch has unnecessary changes to ChangeLog file :) clearing r+ flag, since patch has landed