28876 2009-09-01 09:00:30 -0700 [Qt] reduce peak memory consumption of text decoding. 2009-10-21 20:06:36 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC S60 3rd edition RESOLVED FIXED Qt P2 Normal --- 1 yongjun.zhang webkit-unassigned darin eric hausmann kenneth laszlo.gombos oldest_to_newest 143909 0 yongjun.zhang 2009-09-01 09:00:30 -0700 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. 143924 1 38868 yongjun.zhang 2009-09-01 10:02:15 -0700 Created attachment 38868 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). 144437 2 eric 2009-09-03 01:47:50 -0700 This looks OK to me, but Darin Adler is probably the best person to review this (small) change. 144451 3 38868 ariya.hidayat 2009-09-03 02:06:32 -0700 Comment on attachment 38868 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. 147567 4 39652 yongjun.zhang 2009-09-16 11:25:59 -0700 Created attachment 39652 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. 147568 5 39653 yongjun.zhang 2009-09-16 11:30:37 -0700 Created attachment 39653 Oops! the previous patch has unnecessary changes to ChangeLog file :) 147620 6 kenneth 2009-09-16 14:19:04 -0700 (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? 147622 7 kenneth 2009-09-16 14:21:25 -0700 I personally would find this useful for other devices that are not Symbian based, like one we're currently using for another Nokia product. 149958 8 hausmann 2009-09-25 01:56:26 -0700 Committed r48752: <http://trac.webkit.org/changeset/48752> 156760 9 39653 tonikitoo 2009-10-21 20:06:36 -0700 Comment on attachment 39653 Oops! the previous patch has unnecessary changes to ChangeLog file :) clearing r+ flag, since patch has landed 38868 2009-09-01 10:02:15 -0700 2009-09-16 11:25:59 -0700 Cut big input buffer to small buffers to reduce peak memory during decoding. reduce_decoding_mem.patch text/plain 1805 yongjun.zhang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0Nzk0MSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDktMDktMDEgIFlvbmdqdW4gWmhhbmcgIDx5b25nanVuLnpoYW5n QG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mjg4NzYKKyAgICAg ICAgW1F0XSByZWR1Y2UgcGVhayBtZW1vcnkgY29uc3VtcHRpb24gb2YgdGV4dCBkZWNvZGluZy4K KworICAgICAgICBDaG9wIGxhcmdlIGlucHV0IGJ1ZmZlciBpbnRvIHNtYWxsIGJ1ZmZlcnMgdG8g cmVkdWNlIHBlYWsgbWVtb3J5CisgICAgICAgIGR1cmluZyBkZWNvZGluZy4KKworICAgICAgICAq IHBsYXRmb3JtL3RleHQvcXQvVGV4dENvZGVjUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VGV4 dENvZGVjUXQ6OmRlY29kZSk6CisKIDIwMDktMDktMDEgIEZ1bWl0b3NoaSBVa2FpICA8dWthaUBj aHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQWRhbSBCYXJ0aC4KSW5kZXg6IFdl YkNvcmUvcGxhdGZvcm0vdGV4dC9xdC9UZXh0Q29kZWNRdC5jcHAKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2Vi Q29yZS9wbGF0Zm9ybS90ZXh0L3F0L1RleHRDb2RlY1F0LmNwcAkocmV2aXNpb24gNDc5MzEpCisr KyBXZWJDb3JlL3BsYXRmb3JtL3RleHQvcXQvVGV4dENvZGVjUXQuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC05NCw3ICs5NCwyMiBAQCBUZXh0Q29kZWNRdDo6flRleHRDb2RlY1F0KCkKIAogU3RyaW5n IFRleHRDb2RlY1F0OjpkZWNvZGUoY29uc3QgY2hhciogYnl0ZXMsIHNpemVfdCBsZW5ndGgsIGJv b2wgZmx1c2gsIGJvb2wgLypzdG9wT25FcnJvciovLCBib29sJiBzYXdFcnJvcikKIHsKLSAgICBR U3RyaW5nIHVuaWNvZGUgPSBtX2NvZGVjLT50b1VuaWNvZGUoYnl0ZXMsIGxlbmd0aCwgJm1fc3Rh dGUpOworICAgIC8vIFdlIGNob3AgaW5wdXQgYnVmZmVyIHRvIHNtYWxsZXIgYnVmZmVycyB0byBh dm9pZCBleGNlc3NpdmUgbWVtb3J5IGNvbnN1bXB0aW9uCisgICAgLy8gd2hlbiB0aGUgaW5wdXQg YnVmZmVyIGlzIGJpZy4gIFRoaXMgaGVscHMgcmVkdWNlIHBlYWsgbWVtb3J5IGNvbnN1bXB0aW9u IGluCisgICAgLy8gbW9iaWxlIGRldmljZXMgd2hlcmUgc3lzdGVtIFJBTSBpcyBsaW1pdGVkLgor ICAgIHN0YXRpYyBjb25zdCBpbnQga0lucHV0Q2h1bmtTaXplID0gMzIgKiAxMDI0OworICAgIGNv bnN0IGNoYXIqIGJ1ZiA9IGJ5dGVzOworICAgIGNvbnN0IGNoYXIqIGVuZCA9IGJ1ZiArIGxlbmd0 aDsKKyAgICBTdHJpbmcgdW5pY29kZTsKKworICAgIHdoaWxlIChidWYgPCBlbmQpIHsKKyAgICAg ICAgaW50IHNpemUgPSBlbmQgLSBidWY7CisgICAgICAgIHNpemUgPSBxTWluKHNpemUsIGtJbnB1 dENodW5rU2l6ZSk7CisgICAgICAgIFFTdHJpbmcgZGVjb2RlZCA9IG1fY29kZWMtPnRvVW5pY29k ZShidWYsIHNpemUsICZtX3N0YXRlKTsKKyAgICAgICAgdW5pY29kZS5hcHBlbmQoZGVjb2RlZCk7 CisgICAgICAgIGJ1ZiArPSBzaXplOworICAgIH0KKwogICAgIHNhd0Vycm9yID0gbV9zdGF0ZS5p bnZhbGlkQ2hhcnMgIT0gMDsKIAogICAgIGlmIChmbHVzaCkgewo= 39652 2009-09-16 11:25:59 -0700 2009-09-16 11:30:37 -0700 modified patch as per Ariya's comment. text_decoding.patch text/plain 2649 yongjun.zhang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0ODQyNCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDktMDktMTYgIFlvbmdqdW4gWmhhbmcgIDx5b25nanVuLnpoYW5n QG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mjg4NzYKKyAgICAg ICAgW1F0XSByZWR1Y2UgcGVhayBtZW1vcnkgY29uc3VtcHRpb24gb2YgdGV4dCBkZWNvZGluZy4K KworICAgICAgICBDaG9wIGxhcmdlIGlucHV0IGJ1ZmZlciBpbnRvIHNtYWxsIGJ1ZmZlcnMgdG8g cmVkdWNlIHBlYWsgbWVtb3J5CisgICAgICAgIGR1cmluZyBkZWNvZGluZy4KKworICAgICAgICAq IHBsYXRmb3JtL3RleHQvcXQvVGV4dENvZGVjUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VGV4 dENvZGVjUXQ6OmRlY29kZSk6CisKIDIwMDktMDktMTYgIFZpdGFseSBSZXBlc2hrbyAgPHZpdGFs eXJAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERpbWl0cmkgR2xhemtvdi4K QEAgLTcwMTQsMTAgKzcwMjcsMTAgQEAKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTI4NzYyCiAgICAgICAgIDxyZGFyOi8vcHJvYmxlbS83MTcyNDM3Pgog Ci0gICAgICAgIEEgbWVkaWEgZWxlbWVudCB0cmlnZ2VycyB0aGUgcmVzb3VyY2Ugc2VsZWN0aW9u IGFsZ29yaXRobSBpZsKgCisgICAgICAgIEEgbWVkaWEgZWxlbWVudCB0cmlnZ2VycyB0aGUgcmVz b3VyY2Ugc2VsZWN0aW9uIGFsZ29yaXRobSBpZiAKICAgICAgICAgcGxheSgpIGlzIGNhbGxlZCB3 aGVuIHRoZSBuZXR3b3JrU3RhdGUgYXR0cmlidXRlIGlzIE5FVFdPUktfRU1QVFksIGJ1dAogICAg ICAgICBpdCBhbHNvIGltbWVkaWF0ZWx5IHF1ZXVlcyAncGxheScgYW5kIGVpdGhlciAnd2FpdGlu Zycgb3IgJ3BsYXlpbmcnIGV2ZW50cy4KLSAgICAgICAgT25lIG9mIHRoZSBzdGVwcyBpbiBwcmVw YXJpbmcgdG8gbG9hZCBpcyB0byBmbHVzaCBwZW5kaW5nIGV2ZW50cywgc28gdGhvc2XCoAorICAg ICAgICBPbmUgb2YgdGhlIHN0ZXBzIGluIHByZXBhcmluZyB0byBsb2FkIGlzIHRvIGZsdXNoIHBl bmRpbmcgZXZlbnRzLCBzbyB0aG9zZSAKICAgICAgICAgZXZlbnRzIHdlcmUgbG9zdCBpZiB0aGUg bG9hZCB0aW1lciBmaXJlZCBxdWlja2x5IGVub3VnaC4gRml4ZWQgYnkgZGVsZXRpbmcKICAgICAg ICAgcGVuZGluZyBldmVudHMgYmVmb3JlIHNjaGVkdWxpbmcgdGhlIGxvYWQgdGltZXIuCiAKSW5k ZXg6IFdlYkNvcmUvcGxhdGZvcm0vdGV4dC9xdC9UZXh0Q29kZWNRdC5jcHAKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L3F0L1RleHRDb2RlY1F0LmNwcAkocmV2aXNpb24gNDg0 MjQpCisrKyBXZWJDb3JlL3BsYXRmb3JtL3RleHQvcXQvVGV4dENvZGVjUXQuY3BwCSh3b3JraW5n IGNvcHkpCkBAIC05NCw3ICs5NCwyNiBAQCBUZXh0Q29kZWNRdDo6flRleHRDb2RlY1F0KCkKIAog U3RyaW5nIFRleHRDb2RlY1F0OjpkZWNvZGUoY29uc3QgY2hhciogYnl0ZXMsIHNpemVfdCBsZW5n dGgsIGJvb2wgZmx1c2gsIGJvb2wgLypzdG9wT25FcnJvciovLCBib29sJiBzYXdFcnJvcikKIHsK LSAgICBRU3RyaW5nIHVuaWNvZGUgPSBtX2NvZGVjLT50b1VuaWNvZGUoYnl0ZXMsIGxlbmd0aCwg Jm1fc3RhdGUpOworICAgIC8vIFdlIGNob3AgaW5wdXQgYnVmZmVyIHRvIHNtYWxsZXIgYnVmZmVy cyB0byBhdm9pZCBleGNlc3NpdmUgbWVtb3J5IGNvbnN1bXB0aW9uCisgICAgLy8gd2hlbiB0aGUg aW5wdXQgYnVmZmVyIGlzIGJpZy4gIFRoaXMgaGVscHMgcmVkdWNlIHBlYWsgbWVtb3J5IGNvbnN1 bXB0aW9uIGluCisgICAgLy8gbW9iaWxlIGRldmljZXMgd2hlcmUgc3lzdGVtIFJBTSBpcyBsaW1p dGVkLgorI2lmIFBMQVRGT1JNKFNZTUJJQU4pCisgICAgc3RhdGljIGNvbnN0IGludCBNYXhJbnB1 dENodW5rU2l6ZSA9IDMyICogMTAyNDsKKyNlbHNlCisgICAgc3RhdGljIGNvbnN0IGludCBNYXhJ bnB1dENodW5rU2l6ZSA9IDEwMjQgKiAxMDI0OworI2VuZGlmCisgICAgY29uc3QgY2hhciogYnVm ID0gYnl0ZXM7CisgICAgY29uc3QgY2hhciogZW5kID0gYnVmICsgbGVuZ3RoOworICAgIFN0cmlu ZyB1bmljb2RlOworCisgICAgd2hpbGUgKGJ1ZiA8IGVuZCkgeworICAgICAgICBpbnQgc2l6ZSA9 IGVuZCAtIGJ1ZjsKKyAgICAgICAgc2l6ZSA9IHFNaW4oc2l6ZSwgTWF4SW5wdXRDaHVua1NpemUp OworICAgICAgICBRU3RyaW5nIGRlY29kZWQgPSBtX2NvZGVjLT50b1VuaWNvZGUoYnVmLCBzaXpl LCAmbV9zdGF0ZSk7CisgICAgICAgIHVuaWNvZGUuYXBwZW5kKGRlY29kZWQpOworICAgICAgICBi dWYgKz0gc2l6ZTsKKyAgICB9CisKICAgICBzYXdFcnJvciA9IG1fc3RhdGUuaW52YWxpZENoYXJz ICE9IDA7CiAKICAgICBpZiAoZmx1c2gpIHsK 39653 2009-09-16 11:30:37 -0700 2009-10-21 20:06:36 -0700 Oops! the previous patch has unnecessary changes to ChangeLog file :) text_decoding.patch text/plain 1911 yongjun.zhang SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0ODQyNCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDktMDktMTYgIFlvbmdqdW4gWmhhbmcgIDx5b25nanVuLnpoYW5n QG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mjg4NzYKKyAgICAg ICAgW1F0XSByZWR1Y2UgcGVhayBtZW1vcnkgY29uc3VtcHRpb24gb2YgdGV4dCBkZWNvZGluZy4K KworICAgICAgICBDaG9wIGxhcmdlIGlucHV0IGJ1ZmZlciBpbnRvIHNtYWxsIGJ1ZmZlcnMgdG8g cmVkdWNlIHBlYWsgbWVtb3J5CisgICAgICAgIGR1cmluZyBkZWNvZGluZy4KKworICAgICAgICAq IHBsYXRmb3JtL3RleHQvcXQvVGV4dENvZGVjUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VGV4 dENvZGVjUXQ6OmRlY29kZSk6CisKIDIwMDktMDktMTYgIFZpdGFseSBSZXBlc2hrbyAgPHZpdGFs eXJAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERpbWl0cmkgR2xhemtvdi4K SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vdGV4dC9xdC9UZXh0Q29kZWNRdC5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS90ZXh0L3F0L1RleHRDb2RlY1F0LmNwcAkocmV2aXNpb24g NDg0MjQpCisrKyBXZWJDb3JlL3BsYXRmb3JtL3RleHQvcXQvVGV4dENvZGVjUXQuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC05NCw3ICs5NCwyNiBAQCBUZXh0Q29kZWNRdDo6flRleHRDb2RlY1F0KCkK IAogU3RyaW5nIFRleHRDb2RlY1F0OjpkZWNvZGUoY29uc3QgY2hhciogYnl0ZXMsIHNpemVfdCBs ZW5ndGgsIGJvb2wgZmx1c2gsIGJvb2wgLypzdG9wT25FcnJvciovLCBib29sJiBzYXdFcnJvcikK IHsKLSAgICBRU3RyaW5nIHVuaWNvZGUgPSBtX2NvZGVjLT50b1VuaWNvZGUoYnl0ZXMsIGxlbmd0 aCwgJm1fc3RhdGUpOworICAgIC8vIFdlIGNob3AgaW5wdXQgYnVmZmVyIHRvIHNtYWxsZXIgYnVm ZmVycyB0byBhdm9pZCBleGNlc3NpdmUgbWVtb3J5IGNvbnN1bXB0aW9uCisgICAgLy8gd2hlbiB0 aGUgaW5wdXQgYnVmZmVyIGlzIGJpZy4gIFRoaXMgaGVscHMgcmVkdWNlIHBlYWsgbWVtb3J5IGNv bnN1bXB0aW9uIGluCisgICAgLy8gbW9iaWxlIGRldmljZXMgd2hlcmUgc3lzdGVtIFJBTSBpcyBs aW1pdGVkLgorI2lmIFBMQVRGT1JNKFNZTUJJQU4pCisgICAgc3RhdGljIGNvbnN0IGludCBNYXhJ bnB1dENodW5rU2l6ZSA9IDMyICogMTAyNDsKKyNlbHNlCisgICAgc3RhdGljIGNvbnN0IGludCBN YXhJbnB1dENodW5rU2l6ZSA9IDEwMjQgKiAxMDI0OworI2VuZGlmCisgICAgY29uc3QgY2hhciog YnVmID0gYnl0ZXM7CisgICAgY29uc3QgY2hhciogZW5kID0gYnVmICsgbGVuZ3RoOworICAgIFN0 cmluZyB1bmljb2RlOworCisgICAgd2hpbGUgKGJ1ZiA8IGVuZCkgeworICAgICAgICBpbnQgc2l6 ZSA9IGVuZCAtIGJ1ZjsKKyAgICAgICAgc2l6ZSA9IHFNaW4oc2l6ZSwgTWF4SW5wdXRDaHVua1Np emUpOworICAgICAgICBRU3RyaW5nIGRlY29kZWQgPSBtX2NvZGVjLT50b1VuaWNvZGUoYnVmLCBz aXplLCAmbV9zdGF0ZSk7CisgICAgICAgIHVuaWNvZGUuYXBwZW5kKGRlY29kZWQpOworICAgICAg ICBidWYgKz0gc2l6ZTsKKyAgICB9CisKICAgICBzYXdFcnJvciA9IG1fc3RhdGUuaW52YWxpZENo YXJzICE9IDA7CiAKICAgICBpZiAoZmx1c2gpIHsK