Summary: | [Mac] Avoid creating DOMCSSStyleDeclaration in WebHTMLConverter | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | barraclough, darin, enrica, kling, koivisto, sam | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac (Intel) | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 130227 | ||||||||
Attachments: |
|
Description
Ryosuke Niwa
2014-03-13 23:33:42 PDT
Created attachment 226655 [details]
Extracted HTMLConverterCaches
Created attachment 226657 [details]
A/B test result
It's crazy. This simple C++ rewrite is a 20% improvement.
Comment on attachment 226655 [details] Extracted HTMLConverterCaches View in context: https://bugs.webkit.org/attachment.cgi?id=226655&action=review r=me > Source/WebCore/platform/mac/HTMLConverter.mm:568 > + auto result = m_computedStyles.add(element, nullptr); > + if (result.isNewEntry) > + result.iterator->value = CSSComputedStyleDeclaration::create(element, true); > + CSSStyleDeclaration* declaration = result.iterator->value.get(); This would be more efficient using ComputedStyleExtractor directly. Then we wouldn't have to build a CSSComputedStyleDeclaration object at all. > Source/WebCore/platform/mac/HTMLConverter.mm:569 > + return declaration ? declaration->getPropertyCSSValue(propertyName) : 0; 0 -> nullptr > Source/WebCore/platform/mac/HTMLConverter.mm:577 > + auto result = m_inlineStyles.add(element, nullptr); > + if (result.isNewEntry) > + result.iterator->value = element->style(); > + CSSStyleDeclaration* declaration = result.iterator->value.get(); This code would be more efficient if it accessed the StyleProperties directly via Element::inlineStyle() instead of instantiating a CSSOM wrapper via style(). > Source/WebCore/platform/mac/HTMLConverter.mm:578 > + return declaration ? declaration->getPropertyCSSValue(propertyName) : 0; 0 -> nullptr > Source/WebCore/platform/mac/HTMLConverter.mm:785 > + Element* element = toElement(coreNode); I'd make this Element&. > Source/WebCore/platform/mac/HTMLConverter.mm:1005 > + Element* element = toElement(coreNode); Same here. > Source/WebCore/platform/mac/HTMLConverter.mm:2424 > + _caches = nullptr; Is this actually necessary? (In reply to comment #3) > (From update of attachment 226655 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226655&action=review > > > Source/WebCore/platform/mac/HTMLConverter.mm:568 > > + auto result = m_computedStyles.add(element, nullptr); > > + if (result.isNewEntry) > > + result.iterator->value = CSSComputedStyleDeclaration::create(element, true); > > + CSSStyleDeclaration* declaration = result.iterator->value.get(); > > This would be more efficient using ComputedStyleExtractor directly. Then we wouldn't have to build a CSSComputedStyleDeclaration object at all. Can I store ComputedStyleExtractor in the hash map and use it later? (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 226655 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=226655&action=review > > > > > Source/WebCore/platform/mac/HTMLConverter.mm:568 > > > + auto result = m_computedStyles.add(element, nullptr); > > > + if (result.isNewEntry) > > > + result.iterator->value = CSSComputedStyleDeclaration::create(element, true); > > > + CSSStyleDeclaration* declaration = result.iterator->value.get(); > > > > This would be more efficient using ComputedStyleExtractor directly. Then we wouldn't have to build a CSSComputedStyleDeclaration object at all. > > Can I store ComputedStyleExtractor in the hash map and use it later? Totally. Committed r165635: <http://trac.webkit.org/changeset/165635> |