Bug 186696 – MediaQuerySet wastes a lot of vector capacity

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Simon Fraser (smfr)]

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)

[Simon Fraser (smfr)]

Need to re-do the changes from r204006

[Radar WebKit Bug Importer]

<rdar://problem/41172850>

[Simon Fraser (smfr)]

Tooling in bug 186698.

[Chris Dumez]

Created attachment 342856 [details]
Patch

[Simon Fraser (smfr)]

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.

[Chris Dumez]

(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();
}

[Simon Fraser (smfr)]

Comment on attachment 342856 [details]
Patch

OK

[Chris Dumez]

(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.

[WebKit Commit Bot]

Comment on attachment 342856 [details]
Patch

Clearing flags on attachment: 342856

Committed r232898: <https://trac.webkit.org/changeset/232898>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

*** Bug 186713 has been marked as a duplicate of this bug. ***