Bug 28876

Summary: [Qt] reduce peak memory consumption of text decoding.
Product: WebKit Reporter: Yongjun Zhang <yongjun.zhang>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric, hausmann, kenneth, laszlo.gombos
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: S60 3rd edition   
Attachments:
Description Flags
Cut big input buffer to small buffers to reduce peak memory during decoding.
ariya.hidayat: review-
modified patch as per Ariya's comment.
none
Oops! the previous patch has unnecessary changes to ChangeLog file :) none

Yongjun Zhang
Reported 2009-09-01 09:00:30 PDT
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.
Attachments
Cut big input buffer to small buffers to reduce peak memory during decoding. (1.76 KB, patch)
2009-09-01 10:02 PDT, Yongjun Zhang
ariya.hidayat: review-
modified patch as per Ariya's comment. (2.59 KB, patch)
2009-09-16 11:25 PDT, Yongjun Zhang
no flags
Oops! the previous patch has unnecessary changes to ChangeLog file :) (1.87 KB, patch)
2009-09-16 11:30 PDT, Yongjun Zhang
no flags
Yongjun Zhang
Comment 1 2009-09-01 10:02:15 PDT
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).
Eric Seidel (no email)
Comment 2 2009-09-03 01:47:50 PDT
This looks OK to me, but Darin Adler is probably the best person to review this (small) change.
Ariya Hidayat
Comment 3 2009-09-03 02:06:32 PDT
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.
Yongjun Zhang
Comment 4 2009-09-16 11:25:59 PDT
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.
Yongjun Zhang
Comment 5 2009-09-16 11:30:37 PDT
Created attachment 39653 [details] Oops! the previous patch has unnecessary changes to ChangeLog file :)
Kenneth Rohde Christiansen
Comment 6 2009-09-16 14:19:04 PDT
(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?
Kenneth Rohde Christiansen
Comment 7 2009-09-16 14:21:25 PDT
I personally would find this useful for other devices that are not Symbian based, like one we're currently using for another Nokia product.
Simon Hausmann
Comment 8 2009-09-25 01:56:26 PDT
Antonio Gomes
Comment 9 2009-10-21 20:06:36 PDT
Comment on attachment 39653 [details] Oops! the previous patch has unnecessary changes to ChangeLog file :) clearing r+ flag, since patch has landed
Note You need to log in before you can comment on or make changes to this bug.