Bug 190839 – CSSCalcOperation constructor wastes 6KB of Vector capacity on cnn.com

• Component: New Bugs
• Status: RESOLVED FIXED

[Rob Buis]

Tooling from bug 186698:

Wasted capacity: 6272 bytes (used 896 of 7168 bytes, utilization: 12.50%) - 56 allocations
1   0x10aaf8335 WebCore::CSSCalcExpressionNodeParser::parseCalc(WebCore::CSSParserTokenRange, WebCore::CSSValueID)
2   0x10aaf8212 WebCore::CSSCalcValue::create(WebCore::CSSValueID, WebCore::CSSParserTokenRange const&, WebCore::CalculationCategory, WebCore::ValueRange)
3   0x10abf88a4 WebCore::CSSPropertyParserHelpers::CalcParser::CalcParser(WebCore::CSSParserTokenRange&, WebCore::CalculationCategory, WebCore::ValueRange)
4   0x10abd33fe WebCore::CSSPropertyParserHelpers::consumeLengthOrPercent(WebCore::CSSParserTokenRange&, WebCore::CSSParserMode, WebCore::ValueRange, WebCore::CSSPropertyParserHelpers::UnitlessQuirk)
5   0x10abd3285 WebCore::consumeWidthOrHeight(WebCore::CSSParserTokenRange&, WebCore::CSSParserContext const&, WebCore::CSSPropertyParserHelpers::UnitlessQuirk)
6   0x10abc2dd6 WebCore::CSSPropertyParser::parseSingleValue(WebCore::CSSPropertyID, WebCore::CSSPropertyID)
7   0x10abc22ef WebCore::CSSPropertyParser::parseValueStart(WebCore::CSSPropertyID, bool)
8   0x10abc0923 WebCore::CSSPropertyParser::parseValue(WebCore::CSSPropertyID, bool, WebCore::CSSParserTokenRange const&, WebCore::CSSParserContext const&, WTF::Vector<WebCore::CSSProperty, 256ul, WTF::CrashOnOverflow, 16ul>&, WebCore::StyleRuleBase::Type)

[Rob Buis]

Created attachment 352991 [details]
Patch

[Daniel Bates]

Comment on attachment 352991 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352991&action=review

> Source/WebCore/css/CSSCalculationValue.cpp:665
> +        m_children.reserveInitialCapacity(2);
> +        m_children.uncheckedAppend(WTFMove(leftSide));
> +        m_children.uncheckedAppend(WTFMove(rightSide));

Can we make m_children have inline capacity 2 and then just insert the left and right children using operator[]?

[Rob Buis]

I think it is hard to do that since m_children is also being used by the other constructor as a variable sized Vector, and lots of existing methods rely on m_children (doubleValue, computeLengthPx etc.). Maybe it is possible with lots of refactoring but it may need polymorphism for instance. Probably not worth it IMHO. WDYT?

[Frédéric Wang (:fredw)]

Comment on attachment 352991 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352991&action=review

> Source/WebCore/ChangeLog:3
> +        CSSCalcOperation ctor wastes 6KB of Vector capacity on cnn.com

Can we please write "constructor" everywhere in this patch and for the bug title?

> Source/WebCore/ChangeLog:8
> +        The CSSCalcOperation ctor that takes a left and rightSide parameter

leftSide and rightSide parameters

[Rob Buis]

Created attachment 353854 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 353854 [details]
Patch

Clearing flags on attachment: 353854

Committed r238089: <https://trac.webkit.org/changeset/238089>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45990829>