RESOLVED FIXED 190839
CSSCalcOperation constructor wastes 6KB of Vector capacity on cnn.com 
https://bugs.webkit.org/show_bug.cgi?id=190839
Summary CSSCalcOperation constructor wastes 6KB of Vector capacity on cnn.com
Rob Buis
Reported 2018-10-23 12:41:14 PDT
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)
Attachments
Patch (1.64 KB, patch)
2018-10-23 12:53 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (1.68 KB, patch)
2018-11-05 06:07 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2018-10-23 12:53:39 PDT
Created attachment 352991 [details] Patch
Daniel Bates
Comment 2 2018-10-23 14:06:21 PDT
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
Comment 3 2018-10-23 23:38:41 PDT
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 4 2018-11-05 05:23:07 PST
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
Comment 5 2018-11-05 06:07:37 PST
Created attachment 353854 [details] Patch
WebKit Commit Bot
Comment 6 2018-11-12 08:49:59 PST
Comment on attachment 353854 [details] Patch Clearing flags on attachment: 353854 Committed r238089: <https://trac.webkit.org/changeset/238089>
WebKit Commit Bot
Comment 7 2018-11-12 08:50:00 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-11-12 08:50:26 PST
<rdar://problem/45990829>
Note You need to log in before you can comment on or make changes to this bug.