Bug 186696 - MediaQuerySet wastes a lot of vector capacity
Summary: MediaQuerySet wastes a lot of vector capacity
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
: 186713 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-06-15 14:43 PDT by Simon Fraser (smfr)
Modified: 2018-06-16 16:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.36 KB, patch)
2018-06-15 16:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2018-06-15 14:43:33 PDT
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)
Comment 1 Simon Fraser (smfr) 2018-06-15 14:50:50 PDT
Need to re-do the changes from r204006
Comment 2 Radar WebKit Bug Importer 2018-06-15 14:51:23 PDT
<rdar://problem/41172850>
Comment 3 Simon Fraser (smfr) 2018-06-15 15:00:28 PDT
Tooling in bug 186698.
Comment 4 Chris Dumez 2018-06-15 16:26:42 PDT
Created attachment 342856 [details]
Patch
Comment 5 Simon Fraser (smfr) 2018-06-15 16:29:44 PDT
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.
Comment 6 Chris Dumez 2018-06-15 16:30:42 PDT
(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();
}
Comment 7 Simon Fraser (smfr) 2018-06-15 16:32:44 PDT
Comment on attachment 342856 [details]
Patch

OK
Comment 8 Chris Dumez 2018-06-15 16:36:40 PDT
(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.
Comment 9 WebKit Commit Bot 2018-06-15 17:24:13 PDT
Comment on attachment 342856 [details]
Patch

Clearing flags on attachment: 342856

Committed r232898: <https://trac.webkit.org/changeset/232898>
Comment 10 WebKit Commit Bot 2018-06-15 17:24:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Simon Fraser (smfr) 2018-06-16 16:47:45 PDT
*** Bug 186713 has been marked as a duplicate of this bug. ***