186696 2018-06-15 14:43:33 -0700 MediaQuerySet wastes a lot of vector capacity 2018-06-16 16:47:45 -0700 1 1 1 Unclassified WebKit CSS WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 simon.fraser cdumez cdumez commit-queue koivisto simon.fraser webkit-bug-importer zalan oldest_to_newest 1433560 0 simon.fraser 2018-06-15 14:43:33 -0700 On theverge.com, we waste about half a meg of vector capacity here: Wasted capacity: 589624 bytes (used 39368 of 628992 bytes, utilization: 6.26%) - 702 allocations 1 0x10743f765 WTF::VectorBuffer<WebCore::MediaQuery, 0ul>::VectorBuffer() 2 0x10743f745 WTF::Vector<WebCore::MediaQuery, 0ul, WTF::CrashOnOverflow, 16ul>::Vector() 3 0x1074284f5 WTF::Vector<WebCore::MediaQuery, 0ul, WTF::CrashOnOverflow, 16ul>::Vector() 4 0x1074284b6 WebCore::MediaQuerySet::MediaQuerySet() 5 0x107428515 WebCore::MediaQuerySet::MediaQuerySet() 6 0x1074283be WebCore::MediaQuerySet::create() 7 0x1075404be WebCore::MediaQueryParser::MediaQueryParser(WebCore::MediaQueryParser::ParserType, WebCore::MediaQueryParserContext) 8 0x10753f534 WebCore::MediaQueryParser::MediaQueryParser(WebCore::MediaQueryParser::ParserType, WebCore::MediaQueryParserContext) 1433563 1 simon.fraser 2018-06-15 14:50:50 -0700 Need to re-do the changes from r204006 1433564 2 webkit-bug-importer 2018-06-15 14:51:23 -0700 <rdar://problem/41172850> 1433573 3 simon.fraser 2018-06-15 15:00:28 -0700 Tooling in bug 186698. 1433621 4 342856 cdumez 2018-06-15 16:26:42 -0700 Created attachment 342856 Patch 1433622 5 342856 simon.fraser 2018-06-15 16:29:44 -0700 Comment on attachment 342856 Patch Kling's previous changes here did a lot more shrinking: https://trac.webkit.org/changeset/204006. I also wonder if we should shrink MediaQuery's m_expressions vector. 1433623 6 cdumez 2018-06-15 16:30:42 -0700 (In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 342856 [details] > Patch > > Kling's previous changes here did a lot more shrinking: > https://trac.webkit.org/changeset/204006. I also wonder if we should shrink > MediaQuery's m_expressions vector. The code has not changed and is still there: void MediaQuerySet::shrinkToFit() { m_queries.shrinkToFit(); for (auto& query : m_queries) query.shrinkToFit(); } 1433624 7 342856 simon.fraser 2018-06-15 16:32:44 -0700 Comment on attachment 342856 Patch OK 1433625 8 cdumez 2018-06-15 16:36:40 -0700 (In reply to Chris Dumez from comment #6) > (In reply to Simon Fraser (smfr) from comment #5) > > Comment on attachment 342856 [details] > > Patch > > > > Kling's previous changes here did a lot more shrinking: > > https://trac.webkit.org/changeset/204006. I also wonder if we should shrink > > MediaQuery's m_expressions vector. > > The code has not changed and is still there: > void MediaQuerySet::shrinkToFit() > { > m_queries.shrinkToFit(); > for (auto& query : m_queries) > query.shrinkToFit(); > } I am looking at r204006 but I am not convinced it does more shrinking. We still shrink both the MediaQuerySet and every query in the vector. Also, on ToT, MediaQuerySet::create() is only called in MediaQueryParser, and shrink there after we're done parsing. MediaQuerySet::create(const String&, MediaQueryParserContext&), calls MediaQueryParser::parseMediaQuerySet() which calls MediaQueryParser(MediaQuerySetParser, context).parseInternal(range) where I do the shrinking. I am therefore confident I shrink all MediaQuerySet objects, right after they're parsed. 1433634 9 342856 commit-queue 2018-06-15 17:24:13 -0700 Comment on attachment 342856 Patch Clearing flags on attachment: 342856 Committed r232898: <https://trac.webkit.org/changeset/232898> 1433635 10 commit-queue 2018-06-15 17:24:15 -0700 All reviewed patches have been landed. Closing bug. 1433835 11 simon.fraser 2018-06-16 16:47:45 -0700 *** Bug 186713 has been marked as a duplicate of this bug. *** 342856 2018-06-15 16:26:42 -0700 2018-06-15 17:24:13 -0700 Patch bug-186696-20180615162641.patch text/plain 1397 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjMyODk0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjljMjAzNjE0MzBmOTU4 NTAzYTY3NDA1NGIwMGRlYWY4OWEzMTU2OS4uMTk2NWMwMzI0ZDlhNGQ5ZWI3YThmMzZhNjdhNDQ1 MWZmNDYwOWNjNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE4LTA2LTE1ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgTWVkaWFRdWVyeVNldCB3YXN0 ZXMgYSBsb3Qgb2YgdmVjdG9yIGNhcGFjaXR5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xODY2OTYKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzQxMTcy ODUwPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENh bGwgc2hyaW5rVG9GaXQoKSBvbiB0aGUgTWVkaWFRdWVyeVNldCBhZnRlciB3ZSdyZSBkb25lIHBh cnNpbmcgaXQKKyAgICAgICAgdG8gYXZvaWQgd2FzdGluZyBtZW1vcnkuCisKKyAgICAgICAgKiBj c3MvcGFyc2VyL01lZGlhUXVlcnlQYXJzZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6TWVkaWFR dWVyeVBhcnNlcjo6cGFyc2VJbnRlcm5hbCk6CisKIDIwMTgtMDYtMTUgIEFudHRpIEtvaXZpc3Rv ICA8YW50dGlAYXBwbGUuY29tPgogCiAgICAgICAgIFNlbWFudGljIGNvbG9ycyBzaG91bGQgbm90 IGJlIHRyYW5zZm9ybWVkIGJ5IGNvbG9yLWZpbHRlcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvY3NzL3BhcnNlci9NZWRpYVF1ZXJ5UGFyc2VyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9w YXJzZXIvTWVkaWFRdWVyeVBhcnNlci5jcHAKaW5kZXggNzc4YTliZTY2MWE2Nzk0MTVlNTUwZjEw MmY5ZGVhYzc1MGU1OTVhYy4uNWQzNmYxYjI1ZjVkMGFlNmMyZDhhMDI0ZjA4NWU1MmZmMTExNTdl MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvY3NzL3BhcnNlci9NZWRpYVF1ZXJ5UGFyc2Vy LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvcGFyc2VyL01lZGlhUXVlcnlQYXJzZXIuY3Bw CkBAIC0yNzIsNiArMjcyLDggQEAgUmVmUHRyPE1lZGlhUXVlcnlTZXQ+IE1lZGlhUXVlcnlQYXJz ZXI6OnBhcnNlSW50ZXJuYWwoQ1NTUGFyc2VyVG9rZW5SYW5nZSByYW5nZSkKICAgICB9IGVsc2Ug aWYgKG1fbWVkaWFRdWVyeURhdGEuY3VycmVudE1lZGlhUXVlcnlDaGFuZ2VkKCkpCiAgICAgICAg IGNvbW1pdE1lZGlhUXVlcnkoKTsKIAorICAgIG1fcXVlcnlTZXQtPnNocmlua1RvRml0KCk7CisK ICAgICByZXR1cm4gbV9xdWVyeVNldDsKIH0KIAo=