186709 2018-06-15 15:11:00 -0700 SimpleLineLayout::FlowContents wastes 54KB of Vector capacity on nytimes.com 2018-07-11 06:30:23 -0700 1 1 1 Unclassified WebKit Layout and Rendering WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 simon.fraser zalan bfulgham commit-queue darin saam simon.fraser webkit-bug-importer ysuzuki zalan oldest_to_newest 1433595 0 simon.fraser 2018-06-15 15:11:00 -0700 Tooling from bug 186698: Wasted capacity: 54096 bytes (used 7728 of 61824 bytes, utilization: 12.50%) - 161 allocations 1 0x108927092 WTF::VectorBuffer<WebCore::SimpleLineLayout::FlowContents::Segment, 8ul>::VectorBuffer(unsigned long, unsigned long) 2 0x108926fa8 WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 8ul, WTF::CrashOnOverflow, 16ul>::Vector<0ul, WTF::CrashOnOverflow, 16ul>(WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 0ul, WTF::CrashOnOverflow, 16ul> const&) 3 0x1088faaed WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 8ul, WTF::CrashOnOverflow, 16ul>::Vector<0ul, WTF::CrashOnOverflow, 16ul>(WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 0ul, WTF::CrashOnOverflow, 16ul> const&) 4 0x1088fa790 WebCore::SimpleLineLayout::FlowContents::FlowContents(WebCore::RenderBlockFlow const&) 5 0x1088fab3d WebCore::SimpleLineLayout::FlowContents::FlowContents(WebCore::RenderBlockFlow const&) 6 0x10892f8a6 WebCore::SimpleLineLayout::RunResolver::RunResolver(WebCore::RenderBlockFlow const&, WebCore::SimpleLineLayout::Layout const&) 7 0x10892f9c5 WebCore::SimpleLineLayout::RunResolver::RunResolver(WebCore::RenderBlockFlow const&, WebCore::SimpleLineLayout::Layout const&) 8 0x1088f8309 WebCore::SimpleLineLayout::Layout::runResolver() const 1433597 1 webkit-bug-importer 2018-06-15 15:11:19 -0700 <rdar://problem/41173793> 1433663 2 zalan 2018-06-15 19:31:11 -0700 (In reply to Simon Fraser (smfr) from comment #0) > Tooling from bug 186698: > > Wasted capacity: 54096 bytes (used 7728 of 61824 bytes, utilization: 12.50%) > - 161 allocations > 1 0x108927092 > WTF::VectorBuffer<WebCore::SimpleLineLayout::FlowContents::Segment, > 8ul>::VectorBuffer(unsigned long, unsigned long) > 2 0x108926fa8 > WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 8ul, > WTF::CrashOnOverflow, 16ul>::Vector<0ul, WTF::CrashOnOverflow, > 16ul>(WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 0ul, > WTF::CrashOnOverflow, 16ul> const&) > 3 0x1088faaed > WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 8ul, > WTF::CrashOnOverflow, 16ul>::Vector<0ul, WTF::CrashOnOverflow, > 16ul>(WTF::Vector<WebCore::SimpleLineLayout::FlowContents::Segment, 0ul, > WTF::CrashOnOverflow, 16ul> const&) > 4 0x1088fa790 > WebCore::SimpleLineLayout::FlowContents::FlowContents(WebCore:: > RenderBlockFlow const&) > 5 0x1088fab3d > WebCore::SimpleLineLayout::FlowContents::FlowContents(WebCore:: > RenderBlockFlow const&) > 6 0x10892f8a6 > WebCore::SimpleLineLayout::RunResolver::RunResolver(WebCore::RenderBlockFlow > const&, WebCore::SimpleLineLayout::Layout const&) > 7 0x10892f9c5 > WebCore::SimpleLineLayout::RunResolver::RunResolver(WebCore::RenderBlockFlow > const&, WebCore::SimpleLineLayout::Layout const&) > 8 0x1088f8309 WebCore::SimpleLineLayout::Layout::runResolver() const In initializeSegments(), we call segments.reserveCapacity(numberOfChildren); on the vector -and we recreate it every time the content changes. Is there stacktraces for the wasted allocations? 1433680 3 simon.fraser 2018-06-15 21:53:21 -0700 No, but you could easily add this to the code added in bug 186698. 1439419 4 344335 zalan 2018-07-05 09:49:04 -0700 Created attachment 344335 Patch 1439459 5 344335 commit-queue 2018-07-05 11:18:29 -0700 Comment on attachment 344335 Patch Clearing flags on attachment: 344335 Committed r233530: <https://trac.webkit.org/changeset/233530> 1439460 6 commit-queue 2018-07-05 11:18:30 -0700 All reviewed patches have been landed. Closing bug. 1439573 7 344335 darin 2018-07-05 15:00:02 -0700 Comment on attachment 344335 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344335&action=review > Source/WebCore/ChangeLog:9 > + The size of the m_segments vector in SimpleLineLayoutFlowContents is alway pre-computed and don't change after the initial append. Given that’s true then we could make an even-more-memory efficient implementation by storing a size alongside a std::unique_ptr<Segment[]>. Could even make a class called something like WTF::FixedVector to make that available. Not sure how often we’d use such a thing or how well it would interoperate with things that know how to work with WTF::vector. 1439720 8 344335 ysuzuki 2018-07-06 00:45:56 -0700 Comment on attachment 344335 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344335&action=review >> Source/WebCore/ChangeLog:9 >> + The size of the m_segments vector in SimpleLineLayoutFlowContents is alway pre-computed and don't change after the initial append. > > Given that’s true then we could make an even-more-memory efficient implementation by storing a size alongside a std::unique_ptr<Segment[]>. Could even make a class called something like WTF::FixedVector to make that available. Not sure how often we’d use such a thing or how well it would interoperate with things that know how to work with WTF::vector. std::unique_ptr<T[]> typically allocates bytes for size since `delete [] rawPointer` requires this. So, if we want to have such FixedVector, we should allocate a memory with fastMalloc() internally (instead of having std::unique_ptr<T[]>). 1439802 9 saam 2018-07-06 10:11:56 -0700 Seems like this may be a Speedometer2 regression on iOS 1439803 10 saam 2018-07-06 10:12:18 -0700 (In reply to Saam Barati from comment #9) > Seems like this may be a Speedometer2 regression on iOS About ~3% on certain iOS devices, but not all. 1439805 11 saam 2018-07-06 10:18:41 -0700 (In reply to Saam Barati from comment #10) > (In reply to Saam Barati from comment #9) > > Seems like this may be a Speedometer2 regression on iOS > > About ~3% on certain iOS devices, but not all. Interestingly, JS tests also regressed in this range. So perhaps it's a different change that caused this regression 1441151 12 zalan 2018-07-11 06:30:23 -0700 Committed r233728: <https://trac.webkit.org/changeset/233728> 344335 2018-07-05 09:49:04 -0700 2018-07-05 11:18:29 -0700 Patch bug-186709-20180705094904.patch text/plain 1500 zalan U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzNTIxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZWEzZDM5NDMwN2FiMThj YjFjZjQwNWIyNWE5YmU4YzI5YjFjNDkzYS4uNTNhOGFkMDVmOTkwYjE3MTY0MTcxM2YzNWIzNjdl ZDYyMmViOWMzMCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE4LTA3LTA1ICBaYWxh biBCdWp0YXMgIDx6YWxhbkBhcHBsZS5jb20+CisKKyAgICAgICAgU2ltcGxlTGluZUxheW91dDo6 Rmxvd0NvbnRlbnRzIHdhc3RlcyA1NEtCIG9mIFZlY3RvciBjYXBhY2l0eSBvbiBueXRpbWVzLmNv bQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTg2NzA5 CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS80MTE3Mzc5Mz4KKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgc2l6ZSBvZiB0aGUgbV9zZWdtZW50cyB2 ZWN0b3IgaW4gU2ltcGxlTGluZUxheW91dEZsb3dDb250ZW50cyBpcyBhbHdheSBwcmUtY29tcHV0 ZWQgYW5kIGRvbid0IGNoYW5nZSBhZnRlciB0aGUgaW5pdGlhbCBhcHBlbmQuICAKKworICAgICAg ICBOb3QgdGVzdGFibGUuCisKKyAgICAgICAgKiByZW5kZXJpbmcvU2ltcGxlTGluZUxheW91dEZs b3dDb250ZW50cy5oOgorCiAyMDE4LTA3LTA1ICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5j b20+CiAKICAgICAgICAgQVNTRVJUKG1fZGljdGlvbmFyeVN0YWNrLnNpemUoKSA9PSAxKSBhc3Nl cnRpb24gaGl0IGluIH5LZXllZERlY29kZXJDRigpIHdoZW4gZGVjb2RpbmcgZmFpbHMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9TaW1wbGVMaW5lTGF5b3V0Rmxvd0NvbnRl bnRzLmggYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvU2ltcGxlTGluZUxheW91dEZsb3dDb250 ZW50cy5oCmluZGV4IDJkZWUxYWQyNzM3NGU4ZDYzYjM0YjBkMjg1ZjJjODA5ODIwNjkxYTguLmRl NjNmNjg2YWQxNDdlYTA5OGQ2NGFmMzg1NTk2YWRkYzRjNmNlODAgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3JlbmRlcmluZy9TaW1wbGVMaW5lTGF5b3V0Rmxvd0NvbnRlbnRzLmgKKysrIGIv U291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1NpbXBsZUxpbmVMYXlvdXRGbG93Q29udGVudHMuaApA QCAtNTgsNyArNTgsNyBAQCBwdWJsaWM6CiAKIHByaXZhdGU6CiAgICAgdW5zaWduZWQgc2VnbWVu dEluZGV4Rm9yUnVuU2xvdyh1bnNpZ25lZCBzdGFydCwgdW5zaWduZWQgZW5kKSBjb25zdDsKLSAg ICBjb25zdCBWZWN0b3I8U2VnbWVudCwgOD4gbV9zZWdtZW50czsKKyAgICBjb25zdCBWZWN0b3I8 U2VnbWVudD4gbV9zZWdtZW50czsKICAgICBtdXRhYmxlIHVuc2lnbmVkIG1fbGFzdFNlZ21lbnRJ bmRleCB7IDAgfTsKIH07CiAK