Data collected via bug 86281 show that RuleSet::addToRuleSet() is the most wasteful call site for excess vector capacity.
Hm. This is somewhat surprising since we shrinkToFit() most of the vectors in RuleSet::shrinkToFit(). Something must be going wrong here. :/
Simon, does your logging work correctly when shrinkToFit is used?
Apparently not (you can see the patch in bug 86281).
Previous data was not taking shrinkToFit into account. New data shows addToRuleSet() to be a lot less wasteful, but there is one stack that shows up in the top 20: 117 vectors, 7.11KB used of 73.12KB, 66.02KB wasted at: 1 0x108c1a255 WTF::Vector<WebCore::RuleData, 0ul>::Vector() 2 0x108c07a55 WTF::Vector<WebCore::RuleData, 0ul>::Vector() 3 0x108bfb4e1 WebCore::RuleSet::addToRuleSet(WTF::AtomicStringImpl*, WTF::HashMap<WTF::AtomicStringImpl*, WTF::OwnPtr<WTF::Vector<WebCore::RuleData, 0ul> >, WTF::PtrHash<WTF::AtomicStringImpl*>, WTF::HashTraits<WTF::AtomicStringImpl*>, WTF::HashTraits<WTF::OwnPtr<WTF::Vector<WebCore::RuleData, 0ul> > > >&, WebCore::RuleData const&) 4 0x108bfb6fb WebCore::RuleSet::addRule(WebCore::StyleRule*, WebCore::CSSSelector*, bool, bool, bool) 5 0x108beac71 _ZN7WebCoreL11makeRuleSetERKN3WTF6VectorINS_13StyleResolver11RuleFeatureELm0EEE
Created attachment 141983 [details] Proposed patch
Comment on attachment 141983 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=141983&action=review r=me > Source/WebCore/ChangeLog:3 > + RuleSet::addToRuleSet wastes a lot of Vector capacity. You might want to update the title. This hardly wastes "a lot".
Committed r117084: <http://trac.webkit.org/changeset/117084>
In cases like this where we know the size the RuleSet will be before we create it, it would be more efficient to reserve capacity than to shrink to fit. All we’d have to do is make a RuleSet::create function overload that takes a capacity. What do you guys think of that idea?
Well maybe this is the only call site like that; the point is that you save more memory by not allocating the larger block and shrinking. I suspect that in many cases the memory saved by shrinking after the fact can’t be reused due to fragmentation.
(In reply to comment #8) > In cases like this where we know the size the RuleSet will be before we create it, it would be more efficient to reserve capacity than to shrink to fit. All we’d have to do is make a RuleSet::create function overload that takes a capacity. > > What do you guys think of that idea? I like the idea in principle. If you look at the implementation of RuleSet::shrinkToFit() however, it's nontrivial to predict at RuleSet::create() time how big each subvector should be.
OK. Got it.
One last thought on this: The prediction can be wrong some of the time as long as it’s right most of the time.