108229 2013-01-29 14:30:06 -0800 CSS matched properties cache exploding when mapped SVG attributes are updated frequently 2022-07-13 13:57:14 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED CONFIGURATION CHANGED P2 Normal --- 108345 1 fmalita fmalita bfulgham eric kling koivisto krit pdr schenney oldest_to_newest 819332 0 185308 fmalita 2013-01-29 14:30:06 -0800 Created attachment 185308 Leaks memory, eventually crashes Chromium. https://code.google.com/p/chromium/issues/detail?id=172221 Several issues at play here: 1) the matched properties cache is never swept: StyleResolver::addToMatchedPropertiesCache(): if (++m_matchedPropertiesCacheAdditionsSinceLastSweep >= matchedDeclarationCacheAdditionsBetweenSweeps) { static const unsigned matchedDeclarationCacheSweepTimeInSeconds = 60; m_matchedPropertiesCacheSweepTimer.startOneShot(matchedDeclarationCacheSweepTimeInSeconds); } Since we keep updating the props, the timer keeps getting reset and never fires. I'll post a patch to leave the timer alone if already armed. StyleResolver::matchAllRules( 2) matched SVG properties are cacheable I'm not familiar with this area, but it seems strange that SVG attributes result in cacheable matched properties, while the equivalent CSS test (divs, background-color updates) yields non-cacheable matched properties. For inline style we explicitly make a caching decision: StyleResolver::matchAllRules( // Inline style is immutable as long as there is no CSSOM wrapper. // FIXME: Media control shadow trees seem to have problems with caching. bool isInlineStyleCacheable = !m_styledElement->inlineStyle()->isMutable() && !m_styledElement->isInShadowTree(); while for presentational attributes we never re-evaluate cacheability based on isMutable. I don't understand enough of what's going on here to call it a bug, but it does seem a bit odd to have this asymmetry - someone should confirm that it's intended. 3) SVG matched properties do not get a deterministic matched properties hash key This is because StyledElement::makePresentationAttributeCacheKey() doesn't support SVG attributes: void StyledElement::makePresentationAttributeCacheKey(PresentationAttributeCacheKey& result) const { // FIXME: Enable for SVG. if (namespaceURI() != xhtmlNamespaceURI) return; => SVG StylePropertySets are effectively non-cacheable (in the presentation attribute cache) => rebuildPresentationAttributeStyle() always allocates a new StylePropertySet => the hash computed in StyleResolver::applyMatchedProperties() is always different (is based on MatchedProperties contents which bundles pointers to StylePropertySets) => SVG matched properties are always added to the matched properties cache, even if equivalent entries already exist This probably needs fixing: add SVG support to StyledElement::makePresentationAttributeCacheKey() to enable proper SVG data hashing in both the presentation attribute cache and the matched properties cache. 823176 1 fmalita 2013-02-01 13:12:49 -0800 Andreas, Antti: I'm not familiar with styling and trying to understand what the correct high-level behavior should be here. For a CSS test where I'm modifying 'background-color' on a <div> dynamically, the resulting MatchResult is non-cacheable because of the following snippet in StyleResolver::matchAllRules(): // Now check our inline style attribute. if (m_matchAuthorAndUserStyles && m_styledElement && m_styledElement->inlineStyle()) { // Inline style is immutable as long as there is no CSSOM wrapper. // FIXME: Media control shadow trees seem to have problems with caching. bool isInlineStyleCacheable = !m_styledElement->inlineStyle()->isMutable() && !m_styledElement->isInShadowTree(); // FIXME: Constify. addElementStyleProperties(result, m_styledElement->inlineStyle(), isInlineStyleCacheable); } Is the intent to prevent caching of script-modifiable matched properties? That would make sense, since with a 1 min purge timer on the matched properties cache one could use up lots of memory otherwise. But for SVG attributes there is no such mechanism - the resulting matched properties are cached unconditionally on the presentational attributes path: // Now check author rules, beginning first with presentational attributes mapped from HTML. if (m_styledElement) { addElementStyleProperties(result, m_styledElement->presentationAttributeStyle()); I guess the question is: should SVG attributes always yield non-cacheable matched properties? 1883297 2 bfulgham 2022-07-13 13:57:14 -0700 WebKit seems to use the least amount of memory or other resources (compared to Blink and Gecko). 185308 2013-01-29 14:30:06 -0800 2013-01-29 14:30:06 -0800 Leaks memory, eventually crashes Chromium. leak.html text/html 2322 fmalita PCFET0NUWVBFIEhUTUwgUFVCTElDICItLy9XM0MvL0RURCBIVE1MIDQuMDEvL0VOIiAiaHR0cDov L3d3dy53My5vcmcvVFIvaHRtbDQvc3RyaWN0LmR0ZCI+CjxodG1sPgogICAgPGhlYWQ+CiAgICAg ICAgPG1ldGEgaHR0cC1lcXVpdj0iQ29udGVudC1UeXBlIiBjb250ZW50PSJ0ZXh0L2h0bWw7IGNo YXJzZXQ9dXRmLTgiPgogICAgICAgIDx0aXRsZT5zdmcgdGVzdDwvdGl0bGU+CgoKCiAgICAgICAg PHNjcmlwdCB0eXBlPSJ0ZXh0L2phdmFzY3JpcHQiPgoKICAgICAgICAgICAgdmFyIHN2ZzsKICAg ICAgICAgICAgdmFyIGludGVydmFsOwogICAgICAgICAgICB2YXIgc3ZnOwoKICAgICAgICAgICAg d2luZG93Lm9ubG9hZCA9IGZ1bmN0aW9uKCl7CiAgICAgICAgICAgICAgICBjcmVhdGVTVkcoKTsK ICAgICAgICAgICAgICAgIHN0YXJ0KCk7CiAgICAgICAgICAgIH0KCiAgICAgICAgICAgIGZ1bmN0 aW9uIHN0YXJ0KCl7CiAgICAgICAgICAgICAgICBpbnRlcnZhbCA9IHNldEludGVydmFsKGNyZWF0 ZUVsZW1lbnRzLCAxNik7ICAgICAgICAgICAgICAgIAogICAgICAgICAgICB9CgogICAgICAgICAg ICBmdW5jdGlvbiBjcmVhdGVTVkcoKXsKCiAgICAgICAgICAgICAgICB2YXIgZGl2ID0gZG9jdW1l bnQuZ2V0RWxlbWVudEJ5SWQoInN2Z2RpdiIpOwogICAgICAgICAgICAgICAgZGl2LmlubmVySFRN TCA9ICIiOwoKICAgICAgICAgICAgICAgIHN2ZyA9IGNyZWF0ZVN2Z0VsZW1lbnQoInN2ZyIpOwog ICAgICAgICAgICAgICAgc3ZnLnN0eWxlLnBvc2l0aW9uID0gImFic29sdXRlIjsKICAgICAgICAg ICAgICAgIHN2Zy5zdHlsZS53aWR0aCA9ICI2MDBweCI7CiAgICAgICAgICAgICAgICBzdmcuc3R5 bGUuaGVpZ2h0ID0gIjUwMHB4IjsKICAgICAgICAgICAgICAgIHN2Zy5zZXRBdHRyaWJ1dGUoInZl cnNpb24iLCAiMS4xIik7CiAgICAgICAgICAgICAgICBkaXYuYXBwZW5kQ2hpbGQoc3ZnKTsKICAg ICAgICAgICAgICAgIGNyZWF0ZUVsZW1lbnRzKCk7ICAgICAgICAgICAgICAgCiAgICAgICAgICAg IH0KCgogICAgICAgICAgICBmdW5jdGlvbiBjcmVhdGVFbGVtZW50cygpewoKICAgICAgICAgICAg ICAgIHJlbW92ZUVsZW1lbnRzKCk7CgogICAgICAgICAgICAgICAgZm9yKHZhciBpID0gMDsgaSA8 IDUwMDsgaSsrKXsKICAgICAgICAgICAgICAgICAgICB2YXIgZWxlbWVudCA9IGNyZWF0ZVN2Z0Vs ZW1lbnQoImNpcmNsZSIpOwogICAgICAgICAgICAgICAgICAgIGVsZW1lbnQuc2V0QXR0cmlidXRl KCJyIiwgTWF0aC5yYW5kb20oKSAqIDEwKTsKICAgICAgICAgICAgICAgICAgICB2YXIgdHJhbnNm b3JtID0gInRyYW5zbGF0ZSgiICsgTWF0aC5yb3VuZChNYXRoLnJhbmRvbSgpICogNjAwKSArICIs IiArIE1hdGgucm91bmQoTWF0aC5yYW5kb20oKSAqIDUwMCkgKyAiKSI7CiAgICAgICAgICAgICAg ICAgICAgZWxlbWVudC5zZXRBdHRyaWJ1dGUoInRyYW5zZm9ybSIsIHRyYW5zZm9ybSk7CiAgICAg ICAgICAgICAgICAgICAgc3ZnLmFwcGVuZENoaWxkKGVsZW1lbnQpOwogICAgICAgICAgICAgICAg ICAgIGVsZW1lbnQuc2V0QXR0cmlidXRlKCJmaWxsIiwgIiNDQzAwMDAiKTsKICAgICAgICAgICAg ICAgIH0KICAgICAgICAgICAgfQoKICAgICAgICAgICAgZnVuY3Rpb24gcmVtb3ZlRWxlbWVudHMo KXsKICAgICAgICAgICAgICAgIHdoaWxlKHN2Zy5oYXNDaGlsZE5vZGVzKCkgKXsKICAgICAgICAg ICAgICAgICAgICBzdmcucmVtb3ZlQ2hpbGQoc3ZnLmxhc3RDaGlsZCk7CiAgICAgICAgICAgICAg ICB9ICAgICAgICAgICAgICAgCiAgICAgICAgICAgIH0KCgogICAgICAgICAgICBmdW5jdGlvbiBj cmVhdGVTdmdFbGVtZW50IChuYW1lKSB7CiAgICAgICAgICAgICAgICByZXR1cm4gZG9jdW1lbnQu Y3JlYXRlRWxlbWVudE5TKCJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIsIG5hbWUpOwogICAg ICAgICAgICB9CgogICAgICAgICAgICBmdW5jdGlvbiBzdG9wKCl7CiAgICAgICAgICAgICAgICBj bGVhckludGVydmFsKGludGVydmFsKQogICAgICAgICAgICB9CgoKICAgICAgICA8L3NjcmlwdD4K ICAgIDwvaGVhZD4KICAgIDxib2R5IHN0eWxlPSJiYWNrZ3JvdW5kLWNvbG9yOiNGRkZGRkYiPgog ICAgICAgIDxkaXYgaWQ9InN2Z2RpdiIgc3R5bGU9IndpZHRoOjYwMHB4OyBoZWlnaHQ6NTAwcHg7 Ij48L2Rpdj4KICAgICAgICA8aW5wdXQgdHlwZT0iYnV0dG9uIiB2YWx1ZT0ic3RhcnQiIG9uY2xp Y2s9InN0YXJ0KCkiPgogICAgICAgIDxpbnB1dCB0eXBlPSJidXR0b24iIHZhbHVlPSJzdG9wIiBv bmNsaWNrPSJzdG9wKCkiPgogICAgPC9ib2R5PgogICAgPC9odG1sPgoK