89898 2012-06-25 11:43:59 -0700 Fast path for simple transform parsing 2012-06-25 12:35:55 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 koivisto webkit-unassigned cmarcelo macpherson menard webkit.review.bot oldest_to_newest 656756 0 koivisto 2012-06-25 11:43:59 -0700 When manipulating transforms using script, the transform value parsing can show up in profiles pretty heavily. We can optimize it easily by implementing a fast path that does not spin up the full CSS parser, like we already do for several other common value types. 656769 1 149337 koivisto 2012-06-25 11:53:59 -0700 Created attachment 149337 patch 656770 2 koivisto 2012-06-25 11:55:50 -0700 <rdar://problem/11740369> 656773 3 149337 andersca 2012-06-25 12:01:28 -0700 Comment on attachment 149337 patch View in context: https://bugs.webkit.org/attachment.cgi?id=149337&action=review > Source/WebCore/css/CSSParser.cpp:1009 > +static bool parseTransformArguments(PassRefPtr<WebKitCSSTransformValue> transformValue, CharType* characters, unsigned length, unsigned start, unsigned expectedCount) > +{ I don't think this needs to take a PassRefPtr, just a raw pointer would be fine. > Source/WebCore/css/CSSParser.cpp:1033 > + // Very long strings probably have multiple components which we don't handle here. > + if (string.length() < 13 || string.length() > 32) I'd be nice if you explained the less than check as well. > Source/WebCore/css/CSSParser.cpp:1038 > + UChar c9 = toASCIILowerUnchecked(string[9]); > + UChar c10 = toASCIILowerUnchecked(string[10]); Is it OK to use unchecked here? 656781 4 149337 oliver 2012-06-25 12:14:41 -0700 Comment on attachment 149337 patch View in context: https://bugs.webkit.org/attachment.cgi?id=149337&action=review >> Source/WebCore/css/CSSParser.cpp:1033 >> + if (string.length() < 13 || string.length() > 32) > > I'd be nice if you explained the less than check as well. I don't like the magic numbers here. This almost feels like something we want to auto generate. 656788 5 koivisto 2012-06-25 12:35:23 -0700 http://trac.webkit.org/changeset/121175, with fixes 656789 6 koivisto 2012-06-25 12:35:55 -0700 (In reply to comment #4) > I don't like the magic numbers here. This almost feels like something we want to auto generate. I made them constants. 149337 2012-06-25 11:53:59 -0700 2012-06-25 12:14:41 -0700 patch transform-parsing-fast-path-3.patch text/plain 5125 koivisto SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyMTE3MSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDEyLTA2LTI1ICBBbnR0aSBL b2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KKworICAgICAgICBGYXN0IHBhdGggZm9yIHNpbXBs ZSB0cmFuc2Zvcm0gcGFyc2luZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9ODk4OTgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBXaGVuIG1hbmlwdWxhdGluZyB0cmFuc2Zvcm1zIHVzaW5nIHNjcmlwdCwgdGhl IHRyYW5zZm9ybSB2YWx1ZSBwYXJzaW5nIGNhbiBzaG93IHVwIGluIHByb2ZpbGVzIHByZXR0eSBo ZWF2aWx5IAorICAgICAgICAodXAgNCUgaW4gc29tZSBjYXNlcykuIFdlIGNhbiBvcHRpbWl6ZSBp dCBlYXNpbHkgYnkgaW1wbGVtZW50aW5nIGEgZmFzdCBwYXRoIHRoYXQgZG9lcyBub3Qgc3BpbiB1 cCB0aGUgZnVsbCBDU1MKKyAgICAgICAgcGFyc2VyLCBsaWtlIHdlIGFscmVhZHkgZG8gZm9yIHNl dmVyYWwgb3RoZXIgY29tbW9uIHZhbHVlIHR5cGVzLgorICAgICAgICAKKyAgICAgICAgVGhlIHBh dGNoIGltcGxlbWVudHMgYSBmYXN0IHBhdGggZm9yIHRyYW5zZm9ybSgpLCB0cmFuc2Zvcm1YL1kv WigpIGFuZCB0cmFuc2Zvcm0zRCgpLiBJdCBzcGVlZHMgdXAgcGFyc2luZyBieSA+NXguCisKKyAg ICAgICAgKiBjc3MvQ1NTUGFyc2VyLmNwcDoKKyAgICAgICAgKFdlYkNvcmUpOgorICAgICAgICAo V2ViQ29yZTo6cGFyc2VUcmFuc2Zvcm1Bcmd1bWVudHMpOgorICAgICAgICAoV2ViQ29yZTo6cGFy c2VUcmFuc2Zvcm1WYWx1ZSk6CisgICAgICAgIChXZWJDb3JlOjpDU1NQYXJzZXI6OnBhcnNlVmFs dWUpOgorCiAyMDEyLTA2LTI1ICBBbGxhbiBYYXZpZXIgIDxhbGxhbi54YXZpZXJAdW5kZWZpbmVk bHRkLmNvLnVrPgogCiAgICAgICAgIFtHVEtdIEFkZCBncmFwaHZpeiBkZWJ1ZyBvdXRwdXQgZm9y IHRoZSBnc3RyZWFtZXIgdmlkZW8gcGlwZWxpbmUuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9jc3Mv Q1NTUGFyc2VyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9jc3MvQ1NTUGFyc2Vy LmNwcAkocmV2aXNpb24gMTIxMTIzKQorKysgU291cmNlL1dlYkNvcmUvY3NzL0NTU1BhcnNlci5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTEwMDQsNiArMTAwNCw3MyBAQCBzdGF0aWMgYm9vbCBwYXJz ZUtleXdvcmRWYWx1ZShTdHlsZVByb3BlCiAgICAgcmV0dXJuIHRydWU7CiB9CiAKK3RlbXBsYXRl IDx0eXBlbmFtZSBDaGFyVHlwZT4KK3N0YXRpYyBib29sIHBhcnNlVHJhbnNmb3JtQXJndW1lbnRz KFBhc3NSZWZQdHI8V2ViS2l0Q1NTVHJhbnNmb3JtVmFsdWU+IHRyYW5zZm9ybVZhbHVlLCBDaGFy VHlwZSogY2hhcmFjdGVycywgdW5zaWduZWQgbGVuZ3RoLCB1bnNpZ25lZCBzdGFydCwgdW5zaWdu ZWQgZXhwZWN0ZWRDb3VudCkKK3sKKyAgICB3aGlsZSAoZXhwZWN0ZWRDb3VudCkgeworICAgICAg ICBzaXplX3QgZW5kID0gV1RGOjpmaW5kKGNoYXJhY3RlcnMsIGxlbmd0aCwgZXhwZWN0ZWRDb3Vu dCA9PSAxID8gJyknIDogJywnLCBzdGFydCk7CisgICAgICAgIGlmIChlbmQgPT0gbm90Rm91bmQg fHwgKGV4cGVjdGVkQ291bnQgPT0gMSAmJiBlbmQgIT0gbGVuZ3RoIC0gMSkpCisgICAgICAgICAg ICByZXR1cm4gZmFsc2U7CisgICAgICAgIHVuc2lnbmVkIGFyZ3VtZW50TGVuZ3RoID0gZW5kIC0g c3RhcnQ7CisgICAgICAgIENTU1ByaW1pdGl2ZVZhbHVlOjpVbml0VHlwZXMgdW5pdCA9IENTU1By aW1pdGl2ZVZhbHVlOjpDU1NfTlVNQkVSOworICAgICAgICBkb3VibGUgbnVtYmVyOworICAgICAg ICBpZiAoIXBhcnNlU2ltcGxlTGVuZ3RoKGNoYXJhY3RlcnMgKyBzdGFydCwgYXJndW1lbnRMZW5n dGgsIHVuaXQsIG51bWJlcikpCisgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgICAgIGlm ICh1bml0ICE9IENTU1ByaW1pdGl2ZVZhbHVlOjpDU1NfUFggJiYgKG51bWJlciB8fCB1bml0ICE9 IENTU1ByaW1pdGl2ZVZhbHVlOjpDU1NfTlVNQkVSKSkKKyAgICAgICAgICAgIHJldHVybiBmYWxz ZTsKKyAgICAgICAgdHJhbnNmb3JtVmFsdWUtPmFwcGVuZChjc3NWYWx1ZVBvb2woKS5jcmVhdGVW YWx1ZShudW1iZXIsIHVuaXQpKTsKKyAgICAgICAgc3RhcnQgPSBlbmQgKyAxOworICAgICAgICAt LWV4cGVjdGVkQ291bnQ7CisgICAgfQorICAgIHJldHVybiB0cnVlOworfQorCitzdGF0aWMgYm9v bCBwYXJzZVRyYW5zZm9ybVZhbHVlKFN0eWxlUHJvcGVydHlTZXQqIHByb3BlcnRpZXMsIENTU1By b3BlcnR5SUQgcHJvcGVydHlJRCwgY29uc3QgU3RyaW5nJiBzdHJpbmcsIGJvb2wgaW1wb3J0YW50 KQoreworICAgIGlmIChwcm9wZXJ0eUlEICE9IENTU1Byb3BlcnR5V2Via2l0VHJhbnNmb3JtKQor ICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgLy8gVmVyeSBsb25nIHN0cmluZ3MgcHJvYmFibHkg aGF2ZSBtdWx0aXBsZSBjb21wb25lbnRzIHdoaWNoIHdlIGRvbid0IGhhbmRsZSBoZXJlLgorICAg IGlmIChzdHJpbmcubGVuZ3RoKCkgPCAxMyB8fCBzdHJpbmcubGVuZ3RoKCkgPiAzMikKKyAgICAg ICAgcmV0dXJuIGZhbHNlOworICAgIGlmICghc3RyaW5nLnN0YXJ0c1dpdGgoInRyYW5zbGF0ZSIs IGZhbHNlKSkKKyAgICAgICAgcmV0dXJuIGZhbHNlOworICAgIFVDaGFyIGM5ID0gdG9BU0NJSUxv d2VyVW5jaGVja2VkKHN0cmluZ1s5XSk7CisgICAgVUNoYXIgYzEwID0gdG9BU0NJSUxvd2VyVW5j aGVja2VkKHN0cmluZ1sxMF0pOworCisgICAgV2ViS2l0Q1NTVHJhbnNmb3JtVmFsdWU6OlRyYW5z Zm9ybU9wZXJhdGlvblR5cGUgdHJhbnNmb3JtVHlwZTsKKyAgICB1bnNpZ25lZCBleHBlY3RlZEFy Z3VtZW50Q291bnQgPSAxOworICAgIHVuc2lnbmVkIGFyZ3VtZW50U3RhcnQgPSAxMTsKKyAgICBp ZiAoYzkgPT0gJ3gnICYmIGMxMCA9PSAnKCcpCisgICAgICAgIHRyYW5zZm9ybVR5cGUgPSBXZWJL aXRDU1NUcmFuc2Zvcm1WYWx1ZTo6VHJhbnNsYXRlWFRyYW5zZm9ybU9wZXJhdGlvbjsKKyAgICBl bHNlIGlmIChjOSA9PSAneScgJiYgYzEwID09ICcoJykKKyAgICAgICAgdHJhbnNmb3JtVHlwZSA9 IFdlYktpdENTU1RyYW5zZm9ybVZhbHVlOjpUcmFuc2xhdGVZVHJhbnNmb3JtT3BlcmF0aW9uOwor ICAgIGVsc2UgaWYgKGM5ID09ICd6JyAmJiBjMTAgPT0gJygnKQorICAgICAgICB0cmFuc2Zvcm1U eXBlID0gV2ViS2l0Q1NTVHJhbnNmb3JtVmFsdWU6OlRyYW5zbGF0ZVpUcmFuc2Zvcm1PcGVyYXRp b247CisgICAgZWxzZSBpZiAoYzkgPT0gJygnKSB7CisgICAgICAgIHRyYW5zZm9ybVR5cGUgPSBX ZWJLaXRDU1NUcmFuc2Zvcm1WYWx1ZTo6VHJhbnNsYXRlVHJhbnNmb3JtT3BlcmF0aW9uOworICAg ICAgICBleHBlY3RlZEFyZ3VtZW50Q291bnQgPSAyOworICAgICAgICBhcmd1bWVudFN0YXJ0ID0g MTA7CisgICAgfSBlbHNlIGlmIChjOSA9PSAnMycgJiYgYzEwID09ICdkJyAmJiBzdHJpbmdbMTFd ID09ICcoJykgeworICAgICAgICB0cmFuc2Zvcm1UeXBlID0gV2ViS2l0Q1NTVHJhbnNmb3JtVmFs dWU6OlRyYW5zbGF0ZTNEVHJhbnNmb3JtT3BlcmF0aW9uOworICAgICAgICBleHBlY3RlZEFyZ3Vt ZW50Q291bnQgPSAzOworICAgICAgICBhcmd1bWVudFN0YXJ0ID0gMTI7CisgICAgfSBlbHNlCisg ICAgICAgIHJldHVybiBmYWxzZTsKKworICAgIFJlZlB0cjxXZWJLaXRDU1NUcmFuc2Zvcm1WYWx1 ZT4gdHJhbnNmb3JtVmFsdWUgPSBXZWJLaXRDU1NUcmFuc2Zvcm1WYWx1ZTo6Y3JlYXRlKHRyYW5z Zm9ybVR5cGUpOworICAgIGJvb2wgc3VjY2VzczsKKyAgICBpZiAoc3RyaW5nLmlzOEJpdCgpKQor ICAgICAgICBzdWNjZXNzID0gcGFyc2VUcmFuc2Zvcm1Bcmd1bWVudHModHJhbnNmb3JtVmFsdWUs IHN0cmluZy5jaGFyYWN0ZXJzOCgpLCBzdHJpbmcubGVuZ3RoKCksIGFyZ3VtZW50U3RhcnQsIGV4 cGVjdGVkQXJndW1lbnRDb3VudCk7CisgICAgZWxzZQorICAgICAgICBzdWNjZXNzID0gcGFyc2VU cmFuc2Zvcm1Bcmd1bWVudHModHJhbnNmb3JtVmFsdWUsIHN0cmluZy5jaGFyYWN0ZXJzMTYoKSwg c3RyaW5nLmxlbmd0aCgpLCBhcmd1bWVudFN0YXJ0LCBleHBlY3RlZEFyZ3VtZW50Q291bnQpOwor ICAgIGlmICghc3VjY2VzcykKKyAgICAgICAgcmV0dXJuIGZhbHNlOworICAgIFJlZlB0cjxDU1NW YWx1ZUxpc3Q+IHJlc3VsdCA9IENTU1ZhbHVlTGlzdDo6Y3JlYXRlU3BhY2VTZXBhcmF0ZWQoKTsK KyAgICByZXN1bHQtPmFwcGVuZCh0cmFuc2Zvcm1WYWx1ZS5yZWxlYXNlKCkpOworICAgIHByb3Bl cnRpZXMtPmFkZFBhcnNlZFByb3BlcnR5KENTU1Byb3BlcnR5KENTU1Byb3BlcnR5V2Via2l0VHJh bnNmb3JtLCByZXN1bHQucmVsZWFzZSgpLCBpbXBvcnRhbnQpKTsKKyAgICByZXR1cm4gdHJ1ZTsK K30KKwogUGFzc1JlZlB0cjxDU1NWYWx1ZUxpc3Q+IENTU1BhcnNlcjo6cGFyc2VGb250RmFjZVZh bHVlKGNvbnN0IEF0b21pY1N0cmluZyYgc3RyaW5nKQogewogICAgIGlmIChzdHJpbmcuaXNFbXB0 eSgpKQpAQCAtMTAyMyw2ICsxMDkwLDggQEAgYm9vbCBDU1NQYXJzZXI6OnBhcnNlVmFsdWUoU3R5 bGVQcm9wZXJ0eQogICAgICAgICByZXR1cm4gdHJ1ZTsKICAgICBpZiAocGFyc2VLZXl3b3JkVmFs dWUoZGVjbGFyYXRpb24sIHByb3BlcnR5SUQsIHN0cmluZywgaW1wb3J0YW50LCBjb250ZXh0U3R5 bGVTaGVldC0+cGFyc2VyQ29udGV4dCgpKSkKICAgICAgICAgcmV0dXJuIHRydWU7CisgICAgaWYg KHBhcnNlVHJhbnNmb3JtVmFsdWUoZGVjbGFyYXRpb24sIHByb3BlcnR5SUQsIHN0cmluZywgaW1w b3J0YW50KSkKKyAgICAgICAgcmV0dXJuIHRydWU7CiAKICAgICBDU1NQYXJzZXJDb250ZXh0IGNv bnRleHQoY3NzUGFyc2VyTW9kZSk7CiAgICAgaWYgKGNvbnRleHRTdHlsZVNoZWV0KSB7Cg==