Bug 75569

Summary: DOM Attribute tests on Dromaeo spends 2.7% of time in hasSelectorForAttribute
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: adamk, darin, kling, koivisto, oliver, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Checks needsStyleRecalc() first none

Ryosuke Niwa
Reported 2012-01-04 13:38:43 PST
Go to http://dromaeo.com/?dom and profiled WebKit r103959 while it was running DOM Attributes tests. Besides AtomicString::addSlowCase which accounts for about 12% of time, we spend 2.7% of time in hasSelectorForAttribute. We can probably use bloom filter there as anttik suggested. 6.6% 6.6% JavaScriptCore JSC::IdentifierTable::add(WTF::StringImpl*) 5.8% 5.8% JavaScriptCore WTF::AtomicString::addSlowCase(WTF::StringImpl*) 5.1% 5.1% WebCore WTF::HashTableIterator<WTF::StringImpl*, std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> >, WTF::PairFirstExtractor<std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> > >, WTF::StringHash, WTF::PairHashTraits<WTF::HashTraits<WTF::StringImpl*>, WTF::HashTraits<JSC::Weak<JSC::JSString> > >, WTF::HashTraits<WTF::StringImpl*> > WTF::HashTable<WTF::StringImpl*, std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> >, WTF::PairFirstExtractor<std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> > >, WTF::StringHash, WTF::PairHashTraits<WTF::HashTraits<WTF::StringImpl*>, WTF::HashTraits<JSC::Weak<JSC::JSString> > >, WTF::HashTraits<WTF::StringImpl*> >::find<WTF::IdentityHashTranslator<WTF::StringHash>, WTF::StringImpl*>(WTF::StringImpl* const&) 0.0% 5.0% WebCore WebCore::jsString(JSC::ExecState*, WTF::String const&) 0.0% 0.1% WebCore WebCore::jsHTMLElementId(JSC::ExecState*, JSC::JSValue, JSC::Identifier const&) 0.0% 0.0% WebCore WebCore::jsStringOrNull(JSC::ExecState*, WTF::String const&) 3.5% 3.5% WebCore WebCore::JSHTMLElement::getOwnPropertySlot(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::PropertySlot&) 3.5% 3.5% JavaScriptCore JSC::MarkedBlock::sweep(JSC::MarkedBlock::SweepMode) 3.1% 3.1% Unknown Library 0x2becb9401640 [23.2KB] 2.9% 2.9% JavaScriptCore JSC::JSValue::toPrimitiveString(JSC::ExecState*) const 2.7% 2.7% WebCore bool WTF::HashTable<WTF::AtomicStringImpl*, WTF::AtomicStringImpl*, WTF::IdentityExtractor, WTF::PtrHash<WTF::AtomicStringImpl*>, WTF::HashTraits<WTF::AtomicStringImpl*>, WTF::HashTraits<WTF::AtomicStringImpl*> >::contains<WTF::IdentityHashTranslator<WTF::PtrHash<WTF::AtomicStringImpl*> >, WTF::AtomicStringImpl*>(WTF::AtomicStringImpl* const&) const 0.0% 2.7% WebCore WebCore::CSSStyleSelector::hasSelectorForAttribute(WTF::AtomicString const&) const 0.0% 2.7% WebCore WebCore::Element::recalcStyleIfNeededAfterAttributeChanged(WebCore::Attribute*) 0.0% 2.7% WebCore WebCore::StyledElement::attributeChanged(WebCore::Attribute*, bool) 0.0% 2.7% WebCore WebCore::Element::didModifyAttribute(WebCore::Attribute*) 0.0% 1.8% WebCore WebCore::Element::setAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) 0.0% 1.0% WebCore WebCore::Element::setAttribute(WTF::AtomicString const&, WTF::AtomicString const&, int&) 0.0% 0.0% WebCore WebCore::Element::recalcStyleIfNeededAfterAttributeChanged(WebCore::Attribute*)
Attachments
Checks needsStyleRecalc() first (1.24 KB, patch)
2012-01-04 22:17 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-01-04 21:47:04 PST
It seems like just checking needsStyleRecalc() first suffice to improve the performance here. After adding the check, we get: 6.1% 6.1% JavaScriptCore JSC::IdentifierTable::add(WTF::StringImpl*) 5.4% 5.4% JavaScriptCore WTF::AtomicString::addSlowCase(WTF::StringImpl*) 4.7% 4.7% WebCore WTF::HashTableIterator<WTF::StringImpl*, std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> >, WTF::PairFirstExtractor<std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> > >, WTF::StringHash, WTF::PairHashTraits<WTF::HashTraits<WTF::StringImpl*>, WTF::HashTraits<JSC::Weak<JSC::JSString> > >, WTF::HashTraits<WTF::StringImpl*> > WTF::HashTable<WTF::StringImpl*, std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> >, WTF::PairFirstExtractor<std::pair<WTF::StringImpl*, JSC::Weak<JSC::JSString> > >, WTF::StringHash, WTF::PairHashTraits<WTF::HashTraits<WTF::StringImpl*>, WTF::HashTraits<JSC::Weak<JSC::JSString> > >, WTF::HashTraits<WTF::StringImpl*> >::find<WTF::IdentityHashTranslator<WTF::StringHash>, WTF::StringImpl*>(WTF::StringImpl* const&) 3.6% 3.6% Unknown Library 0x5c76577ee160 [11.3KB] 3.1% 3.1% WebCore WebCore::JSHTMLElement::getOwnPropertySlot(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::PropertySlot&) 3.1% 3.1% JavaScriptCore JSC::MarkedBlock::sweep(JSC::MarkedBlock::SweepMode) 3.0% 3.0% WebCore WebCore::jsString(JSC::ExecState*, WTF::String const&) 2.9% 2.9% WebCore WebCore::Element::setAttribute(WTF::AtomicString const&, WTF::AtomicString const&, int&) 2.9% 2.9% JavaScriptCore JSC::JSValue::toPrimitiveString(JSC::ExecState*) const 2.7% 2.7% WebCore WebCore::JSHTMLHeadingElement::getOwnPropertySlot(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::PropertySlot&) 2.7% 2.7% WebCore JSC::JSValue::toString(JSC::ExecState*) const 2.7% 2.7% JavaScriptCore JSC::JSString::resolveRope(JSC::ExecState*) const 2.6% 2.6% JavaScriptCore JSC::JSValue::get(JSC::ExecState*, JSC::Identifier const&, JSC::PropertySlot&) const 2.4% 2.4% Unknown Library 0x5c76577b2980 [5.3KB] 2.4% 2.4% WebCore WebCore::Element::idAttributeChanged(WebCore::Attribute*) 2.3% 2.3% JavaScriptCore WTF::StringImpl::lower() 2.2% 2.2% WebCore WebCore::Element::getAttribute(WebCore::QualifiedName const&) const 2.2% 2.2% WebCore WebCore::jsElementPrototypeFunctionSetAttribute(JSC::ExecState*) 2.0% 2.0% JavaScriptCore WTF::AtomicString::lower() const 1.9% 1.9% WebCore WebCore::jsElementPrototypeFunctionGetAttribute(JSC::ExecState*) 1.9% 1.9% WebCore WebCore::StyledElement::attributeChanged(WebCore::Attribute*, bool) 1.8% 1.8% JavaScriptCore JSC::JSString::destroy(JSC::JSCell*) 1.7% 1.7% JavaScriptCore WTF::tryFastMalloc(unsigned long) 1.7% 1.7% WebCore WebCore::Element::setAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) 1.6% 1.6% WebCore WebCore::JSHTMLAnchorElement::put(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&) 1.3% 1.3% Unknown Library 0x5c7657646900 [5.4KB] 1.3% 1.3% JavaScriptCore JSC::JSObject::put(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&) 1.2% 1.2% WebCore WebCore::JSHTMLElement::put(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&) 1.2% 1.2% WebCore WebCore::HTMLAnchorElement::parseMappedAttribute(WebCore::Attribute*) 1.0% 1.0% JavaScriptCore operationGetByVal 1.0% 1.0% Unknown Library 0x5c765761a200 [1.9KB] 1.0% 1.0% JavaScriptCore cti_op_put_by_val
Ryosuke Niwa
Comment 2 2012-01-04 22:17:51 PST
Created attachment 121215 [details] Checks needsStyleRecalc() first
Andreas Kling
Comment 3 2012-01-04 22:19:59 PST
Comment on attachment 121215 [details] Checks needsStyleRecalc() first Nice!
WebKit Review Bot
Comment 4 2012-01-05 08:00:27 PST
Comment on attachment 121215 [details] Checks needsStyleRecalc() first Clearing flags on attachment: 121215 Committed r104165: <http://trac.webkit.org/changeset/104165>
WebKit Review Bot
Comment 5 2012-01-05 08:00:32 PST
All reviewed patches have been landed. Closing bug.
Antti Koivisto
Comment 6 2012-01-05 08:06:35 PST
How much did this patch actually improve the performance?
Ryosuke Niwa
Comment 7 2012-01-05 16:51:43 PST
It appears that there's some statistically significant improvement. with patch: 537.46 runs/s 539.04 runs/s 523.74 runs/s 533.79 runs/s 544.72 runs/s 541.45 runs/s 518.91 runs/s avg: 534.16 stdev: 9.49 (1.78%) without patch: 529.29 runs/s 517.19 runs/s 512.61 runs/s 515.45 runs/s 529.42 runs/s 517.45 runs/s 508.90 runs/s avg: 518.62 stdev: 2.90 (1.52%) Improvement: 2.91%
Ryosuke Niwa
Comment 8 2012-01-05 17:08:15 PST
The scores were measured on r103634 (Apple's mac port); btw, the measured performance gain appears to match the profiling result so we've basically eliminated the time spent inside hasSelectorForAttribute :D.
Note You need to log in before you can comment on or make changes to this bug.