129851 2014-03-06 17:28:21 -0800 Add fast path for id/name/style/class in getAttribute/setAttribute bindings. 2014-05-12 10:08:07 -0700 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID P2 Normal --- 1 kling kling cdumez cmarcelo commit-queue dongseong.hwang esprehn+autocc kangil.han kling kondapallykalyan rniwa oldest_to_newest 987960 0 kling 2014-03-06 17:28:21 -0800 Add fast path for id/name/style/class in getAttribute/setAttribute bindings. 987962 1 226068 kling 2014-03-06 17:30:38 -0800 Created attachment 226068 Patch 987969 2 rniwa 2014-03-06 17:44:42 -0800 Can we make sure we haven't regressed other attar accessed significantly with this? 989027 3 dongseong.hwang 2014-03-10 12:42:18 -0700 Hi, I'm cherry-picking this to Blink, https://codereview.chromium.org/189483004/ In the bug, I measured performance. Both were run on my Ivy Bridge laptop. PerformanceTests/Bindings/get-attribute.html -> 16% improvement Before : Avg get-attribute: 273.565932runs/s After : Avg get-attribute: 317.818405runs/s PerformanceTests/Bindings/set-attribute.html -> 77% improvement Before : Avg set-attribute: 476.101349runs/s After : Avg set-attribute: 847.042662runs/s 989028 4 226068 dongseong.hwang 2014-03-10 12:43:15 -0700 Comment on attachment 226068 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226068&action=review > Source/WebCore/dom/Element.cpp:954 > +void Element::bindingsSetAttribute(const String& localName, const String& newValue, ExceptionCode& ec) Could I ask why you used String& instead of AtomicString&? 989822 5 dongseong.hwang 2014-03-12 13:49:32 -0700 Blink gave up cherry-picking this patch because it causes regression of other attributes significantly (about 50%). getAttribute("id") -> 11% improvement Before : Avg get-attribute: 329.751776runs/s After : Avg get-attribute: 367.252965runs/s getAttribute("group") -> 51% regression Before : Avg get-attribute-href: 329.339992runs/s After : Avg get-attribute-href: 159.540438runs/s setAttribute("id") -> 19% improvement Before : Avg set-attribute: 809.784217runs/s After : Avg set-attribute: 965.404519runs/s setAttribute("group") -> 45% regression Avg set-attribute-href: 756.714409runs/s Avg set-attribute-href: 415.186785runs/s 989830 6 cdumez 2014-03-12 14:05:22 -0700 (In reply to comment #5) > Blink gave up cherry-picking this patch because it causes regression of other attributes significantly (about 50%). > > getAttribute("id") -> 11% improvement > Before : Avg get-attribute: 329.751776runs/s > After : Avg get-attribute: 367.252965runs/s > > getAttribute("group") -> 51% regression > Before : Avg get-attribute-href: 329.339992runs/s > After : Avg get-attribute-href: 159.540438runs/s > > setAttribute("id") -> 19% improvement > Before : Avg set-attribute: 809.784217runs/s > After : Avg set-attribute: 965.404519runs/s > > setAttribute("group") -> 45% regression > Avg set-attribute-href: 756.714409runs/s > Avg set-attribute-href: 415.186785runs/s Dongseong, are those the results with Blink or the results with WebKit? Results may be different. 989835 7 rniwa 2014-03-12 14:11:36 -0700 Note that we didn't land this patch. 1008633 8 kling 2014-05-12 10:08:07 -0700 Oh this. We don't need this now that we construct perfect strings in JSC. 226068 2014-03-06 17:30:38 -0800 2014-03-10 12:43:15 -0700 Patch bug-129851.diff text/plain 4912 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA3NTQyMjI1Li4yY2VlYTQ1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjYg QEAKKzIwMTQtMDMtMDYgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgorCisgICAg ICAgIEFkZCBmYXN0IHBhdGggZm9yIGlkL25hbWUvc3R5bGUvY2xhc3MgaW4gZ2V0QXR0cmlidXRl L3NldEF0dHJpYnV0ZSBiaW5kaW5ncy4KKyAgICAgICAgPGh0dHBzOi8vd2Via2l0Lm9yZy9iLzEy OTg1MT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBT aW5jZSB0aGVzZSBhdHRyaWJ1dGVzIGFyZSBzbyBjb21tb25seSByZWFkIGFuZCBtb2RpZmllZCB2 aWEgdGhlIERPTSBiaW5kaW5ncywKKyAgICAgICAgYWRkIGEgbGl0dGxlIGZhc3QgcGF0aCB0byBh dm9pZCBkb2luZyBBdG9taWNTdHJpbmcgbG9va3VwcyBmb3IgdGhvc2UgbmFtZXMKKyAgICAgICAg d2hlbiB0aGV5IGFyZSB1c2VkIGluIGZyaWVuZGx5IGxvd2VyY2FzZS4KKworICAgICAgICAqIGRv bS9FbGVtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmZhc3RBdHRyaWJ1dGVOYW1lRnJvbVN0 cmluZyk6CisKKyAgICAgICAgICAgIEhlbHBlciB0aGF0IHRyaWVzIHRvIHR1cm4gYSBTdHJpbmcg aW50byBhbiBhdHRyaWJ1dGUgbmFtZS4KKworICAgICAgICAqIGRvbS9FbGVtZW50Lmg6CisgICAg ICAgICogZG9tL0VsZW1lbnQuaWRsOgorICAgICAgICAoV2ViQ29yZTo6RWxlbWVudDo6YmluZGlu Z3NHZXRBdHRyaWJ1dGUpOgorICAgICAgICAoV2ViQ29yZTo6RWxlbWVudDo6YmluZGluZ3NTZXRB dHRyaWJ1dGUpOgorCisgICAgICAgICAgICBBZGRlZCBiaW5kaW5ncyBmdW5jdGlvbnMgdGhhdCB0 YWtlIFN0cmluZyBpbnN0ZWFkIG9mIEF0b21pY1N0cmluZy4KKwogMjAxNC0wMy0wNiAgQnJpYW4g QnVyZyAgPGJidXJnQGFwcGxlLmNvbT4KIAogICAgICAgICBXZWIgUmVwbGF5OiBwcmVtYXR1cmUg cmVsZWFzZSgpIG9mIFBhc3NSZWZQdHIgaW4gSW5zcGVjdG9yUmVwbGF5QWdlbnQKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2RvbS9F bGVtZW50LmNwcAppbmRleCAxYTQxNjk1Li4xMjI4ZTFmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9kb20vRWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZG9tL0VsZW1lbnQuY3Bw CkBAIC05MjQsNiArOTI0LDQzIEBAIEludFJlY3QgRWxlbWVudDo6c2NyZWVuUmVjdCgpIGNvbnN0 CiAgICAgcmV0dXJuIEludFJlY3QoKTsKIH0KIAorc3RhdGljIGlubGluZSBjb25zdCBRdWFsaWZp ZWROYW1lJiBmYXN0QXR0cmlidXRlTmFtZUZyb21TdHJpbmcoY29uc3QgU3RyaW5nJiBsb2NhbE5h bWUpCit7CisgICAgdW5zaWduZWQgbGVuZ3RoID0gbG9jYWxOYW1lLmxlbmd0aCgpOworICAgIGlm ICghbGVuZ3RoKQorICAgICAgICByZXR1cm4gbnVsbFFOYW1lKCk7CisgICAgY29uc3QgU3RyaW5n SW1wbCYgbmFtZSA9ICpsb2NhbE5hbWUuaW1wbCgpOworCisgICAgaWYgKGxlbmd0aCA9PSAyICYm IG5hbWVbMF0gPT0gJ2knICYmIG5hbWVbMV0gPT0gJ2QnKQorICAgICAgICByZXR1cm4gSFRNTE5h bWVzOjppZEF0dHI7CisgICAgaWYgKGxlbmd0aCA9PSA0ICYmIG5hbWVbMF0gPT0gJ24nICYmIG5h bWVbMV0gPT0gJ2EnICYmIG5hbWVbMl0gPT0gJ20nICYmIG5hbWVbM10gPT0gJ2UnKQorICAgICAg ICByZXR1cm4gSFRNTE5hbWVzOjpuYW1lQXR0cjsKKyAgICBpZiAobGVuZ3RoID09IDUgJiYgbmFt ZVswXSA9PSAnYycgJiYgbmFtZVsxXSA9PSAnbCcgJiYgbmFtZVsyXSA9PSAnYScgJiYgbmFtZVsz XSA9PSAncycgJiYgbmFtZVs0XSA9PSAncycpCisgICAgICAgIHJldHVybiBIVE1MTmFtZXM6OmNs YXNzQXR0cjsKKyAgICBpZiAobGVuZ3RoID09IDUgJiYgbmFtZVswXSA9PSAncycgJiYgbmFtZVsx XSA9PSAndCcgJiYgbmFtZVsyXSA9PSAneScgJiYgbmFtZVszXSA9PSAnbCcgJiYgbmFtZVs0XSA9 PSAnZScpCisgICAgICAgIHJldHVybiBIVE1MTmFtZXM6OnN0eWxlQXR0cjsKKworICAgIHJldHVy biBudWxsUU5hbWUoKTsKK30KKworY29uc3QgQXRvbWljU3RyaW5nJiBFbGVtZW50OjpiaW5kaW5n c0dldEF0dHJpYnV0ZShjb25zdCBTdHJpbmcmIGxvY2FsTmFtZSkgY29uc3QKK3sKKyAgICBjb25z dCBRdWFsaWZpZWROYW1lJiBmYXN0TmFtZSA9IGZhc3RBdHRyaWJ1dGVOYW1lRnJvbVN0cmluZyhs b2NhbE5hbWUpOworICAgIGlmIChmYXN0TmFtZSAhPSBudWxsUU5hbWUoKSkKKyAgICAgICAgcmV0 dXJuIGZhc3RHZXRBdHRyaWJ1dGUoZmFzdE5hbWUpOworICAgIHJldHVybiBnZXRBdHRyaWJ1dGUo QXRvbWljU3RyaW5nKGxvY2FsTmFtZSkpOworfQorCit2b2lkIEVsZW1lbnQ6OmJpbmRpbmdzU2V0 QXR0cmlidXRlKGNvbnN0IFN0cmluZyYgbG9jYWxOYW1lLCBjb25zdCBTdHJpbmcmIG5ld1ZhbHVl LCBFeGNlcHRpb25Db2RlJiBlYykKK3sKKyAgICBjb25zdCBRdWFsaWZpZWROYW1lJiBmYXN0TmFt ZSA9IGZhc3RBdHRyaWJ1dGVOYW1lRnJvbVN0cmluZyhsb2NhbE5hbWUpOworICAgIGlmIChmYXN0 TmFtZSAhPSBudWxsUU5hbWUoKSkgeworICAgICAgICBzZXRBdHRyaWJ1dGUoZmFzdE5hbWUsIEF0 b21pY1N0cmluZyhuZXdWYWx1ZSkpOworICAgICAgICByZXR1cm47CisgICAgfQorICAgIHNldEF0 dHJpYnV0ZShBdG9taWNTdHJpbmcobG9jYWxOYW1lKSwgQXRvbWljU3RyaW5nKG5ld1ZhbHVlKSwg ZWMpOworfQorCiBjb25zdCBBdG9taWNTdHJpbmcmIEVsZW1lbnQ6OmdldEF0dHJpYnV0ZShjb25z dCBBdG9taWNTdHJpbmcmIGxvY2FsTmFtZSkgY29uc3QKIHsKICAgICBpZiAoIWVsZW1lbnREYXRh KCkpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudC5oIGIvU291cmNlL1dl YkNvcmUvZG9tL0VsZW1lbnQuaAppbmRleCBhMWU3NzRmLi41MWQ3MzEwIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9kb20vRWxlbWVudC5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVt ZW50LmgKQEAgLTE3Niw2ICsxNzYsOSBAQCBwdWJsaWM6CiAgICAgY29uc3QgQXRvbWljU3RyaW5n JiBnZXRBdHRyaWJ1dGUoY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lKSBjb25zdDsKICAgICBjb25z dCBBdG9taWNTdHJpbmcmIGdldEF0dHJpYnV0ZU5TKGNvbnN0IEF0b21pY1N0cmluZyYgbmFtZXNw YWNlVVJJLCBjb25zdCBBdG9taWNTdHJpbmcmIGxvY2FsTmFtZSkgY29uc3Q7CiAKKyAgICBjb25z dCBBdG9taWNTdHJpbmcmIGJpbmRpbmdzR2V0QXR0cmlidXRlKGNvbnN0IFN0cmluZyYgbmFtZSkg Y29uc3Q7CisgICAgdm9pZCBiaW5kaW5nc1NldEF0dHJpYnV0ZShjb25zdCBTdHJpbmcmIG5hbWUs IGNvbnN0IFN0cmluZyYgdmFsdWUsIEV4Y2VwdGlvbkNvZGUmKTsKKwogICAgIHZvaWQgc2V0QXR0 cmlidXRlKGNvbnN0IEF0b21pY1N0cmluZyYgbmFtZSwgY29uc3QgQXRvbWljU3RyaW5nJiB2YWx1 ZSwgRXhjZXB0aW9uQ29kZSYpOwogICAgIHN0YXRpYyBib29sIHBhcnNlQXR0cmlidXRlTmFtZShR dWFsaWZpZWROYW1lJiwgY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lc3BhY2VVUkksIGNvbnN0IEF0 b21pY1N0cmluZyYgcXVhbGlmaWVkTmFtZSwgRXhjZXB0aW9uQ29kZSYpOwogICAgIHZvaWQgc2V0 QXR0cmlidXRlTlMoY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lc3BhY2VVUkksIGNvbnN0IEF0b21p Y1N0cmluZyYgcXVhbGlmaWVkTmFtZSwgY29uc3QgQXRvbWljU3RyaW5nJiB2YWx1ZSwgRXhjZXB0 aW9uQ29kZSYpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL0VsZW1lbnQuaWRsIGIv U291cmNlL1dlYkNvcmUvZG9tL0VsZW1lbnQuaWRsCmluZGV4IGY4ZTkxY2EuLjVhNGE4Y2UgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9FbGVtZW50LmlkbAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9kb20vRWxlbWVudC5pZGwKQEAgLTI2LDggKzI2LDggQEAKIAogICAgIFtUcmVhdFJldHVy bmVkTnVsbFN0cmluZ0FzPU51bGxdIHJlYWRvbmx5IGF0dHJpYnV0ZSBET01TdHJpbmcgdGFnTmFt ZTsKIAotICAgIFtUcmVhdFJldHVybmVkTnVsbFN0cmluZ0FzPU51bGxdIERPTVN0cmluZyBnZXRB dHRyaWJ1dGUoW0RlZmF1bHQ9VW5kZWZpbmVkXSBvcHRpb25hbCBET01TdHJpbmcgbmFtZSk7Ci0g ICAgW09iakNMZWdhY3lVbm5hbWVkUGFyYW1ldGVycywgUmFpc2VzRXhjZXB0aW9uXSB2b2lkIHNl dEF0dHJpYnV0ZShbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIERPTVN0cmluZyBuYW1lLAor ICAgIFtJbXBsZW1lbnRlZEFzPWJpbmRpbmdzR2V0QXR0cmlidXRlLCBUcmVhdFJldHVybmVkTnVs bFN0cmluZ0FzPU51bGxdIERPTVN0cmluZyBnZXRBdHRyaWJ1dGUoW0RlZmF1bHQ9VW5kZWZpbmVk XSBvcHRpb25hbCBET01TdHJpbmcgbmFtZSk7CisgICAgW0ltcGxlbWVudGVkQXM9YmluZGluZ3NT ZXRBdHRyaWJ1dGUsIE9iakNMZWdhY3lVbm5hbWVkUGFyYW1ldGVycywgUmFpc2VzRXhjZXB0aW9u XSB2b2lkIHNldEF0dHJpYnV0ZShbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIERPTVN0cmlu ZyBuYW1lLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFtEZWZhdWx0PVVu ZGVmaW5lZF0gb3B0aW9uYWwgRE9NU3RyaW5nIHZhbHVlKTsKICAgICB2b2lkIHJlbW92ZUF0dHJp YnV0ZShbRGVmYXVsdD1VbmRlZmluZWRdIG9wdGlvbmFsIERPTVN0cmluZyBuYW1lKTsKICAgICBB dHRyIGdldEF0dHJpYnV0ZU5vZGUoW0RlZmF1bHQ9VW5kZWZpbmVkXSBvcHRpb25hbCBET01TdHJp bmcgbmFtZSk7Cg==