75569 2012-01-04 13:38:43 -0800 DOM Attribute tests on Dromaeo spends 2.7% of time in hasSelectorForAttribute 2012-01-05 17:08:15 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 rniwa rniwa adamk darin kling koivisto oliver webkit.review.bot oldest_to_newest 529713 0 rniwa 2012-01-04 13:38:43 -0800 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*) 529999 1 rniwa 2012-01-04 21:47:04 -0800 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 530005 2 121215 rniwa 2012-01-04 22:17:51 -0800 Created attachment 121215 Checks needsStyleRecalc() first 530007 3 121215 kling 2012-01-04 22:19:59 -0800 Comment on attachment 121215 Checks needsStyleRecalc() first Nice! 530285 4 121215 webkit.review.bot 2012-01-05 08:00:27 -0800 Comment on attachment 121215 Checks needsStyleRecalc() first Clearing flags on attachment: 121215 Committed r104165: <http://trac.webkit.org/changeset/104165> 530286 5 webkit.review.bot 2012-01-05 08:00:32 -0800 All reviewed patches have been landed. Closing bug. 530295 6 koivisto 2012-01-05 08:06:35 -0800 How much did this patch actually improve the performance? 530779 7 rniwa 2012-01-05 16:51:43 -0800 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% 530798 8 rniwa 2012-01-05 17:08:15 -0800 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. 121215 2012-01-04 22:17:51 -0800 2012-01-05 08:00:27 -0800 Checks needsStyleRecalc() first bug-75569-20120104221750.patch text/plain 1267 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNDExNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEyLTAxLTA0ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIERPTSBBdHRyaWJ1dGUgdGVzdHMg b24gRHJvbWFlbyBzcGVuZHMgMi43JSBvZiB0aW1lIGluIGhhc1NlbGVjdG9yRm9yQXR0cmlidXRl CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NTU2OQor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENoZWNrIG5l ZWRzU3R5bGVSZWNhbGMoKSBmaXJzdCB0byBhdm9pZCB1bm5lY2Vzc2FyeSBoYXNoIGxvb2t1cHMu CisKKyAgICAgICAgKiBkb20vRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpFbGVtZW50 OjpyZWNhbGNTdHlsZUlmTmVlZGVkQWZ0ZXJBdHRyaWJ1dGVDaGFuZ2VkKToKKwogMjAxMi0wMS0w NCAgU2hlcmlmZiBCb3QgIDx3ZWJraXQucmV2aWV3LmJvdEBnbWFpbC5jb20+CiAKICAgICAgICAg VW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjEwNDA4NC4KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2Rv bS9FbGVtZW50LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudC5j cHAJKHJldmlzaW9uIDEwMzk1OSkKKysrIFNvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50LmNwcAko d29ya2luZyBjb3B5KQpAQCAtNzA2LDYgKzcwNiw5IEBAIHZvaWQgRWxlbWVudDo6dXBkYXRlQWZ0 ZXJBdHRyaWJ1dGVDaGFuZ2UKICAgICAKIHZvaWQgRWxlbWVudDo6cmVjYWxjU3R5bGVJZk5lZWRl ZEFmdGVyQXR0cmlidXRlQ2hhbmdlZChBdHRyaWJ1dGUqIGF0dHIpCiB7CisgICAgaWYgKG5lZWRz U3R5bGVSZWNhbGMoKSkKKyAgICAgICAgcmV0dXJuOworCiAgICAgaWYgKGRvY3VtZW50KCktPmF0 dGFjaGVkKCkgJiYgZG9jdW1lbnQoKS0+c3R5bGVTZWxlY3RvcigpLT5oYXNTZWxlY3RvckZvckF0 dHJpYnV0ZShhdHRyLT5uYW1lKCkubG9jYWxOYW1lKCkpKQogICAgICAgICBzZXROZWVkc1N0eWxl UmVjYWxjKCk7CiB9Cg==