44107 2010-08-17 07:07:26 -0700 CSS: Make rgb() and rgba() fast paths case-insensitive 2010-08-18 13:05:31 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) All All RESOLVED FIXED Performance P2 Normal --- 1 kling webkit-unassigned oldest_to_newest 265517 0 kling 2010-08-17 07:07:26 -0700 The fast path in CSSParser::parseColor() could easily catch RGB() and RGBA() as well. Instead of equalIgnoringCase(), we can do inline character comparison and gain some speed while we're at it, too. 265521 1 64588 kling 2010-08-17 07:13:12 -0700 Created attachment 64588 Proposed patch 265865 2 64588 ariya.hidayat 2010-08-17 17:25:24 -0700 Comment on attachment 64588 Proposed patch > - if (name.startsWith("rgba(")) { > + if (mightBeRGBA(characters, length)) { Will there be a (minor) behavior difference IF the input is invalid e.g. "rgba(foo"? 265872 3 kling 2010-08-17 17:36:54 -0700 (In reply to comment #2) > (From update of attachment 64588 [details]) > > - if (name.startsWith("rgba(")) { > > + if (mightBeRGBA(characters, length)) { > > Will there be a (minor) behavior difference IF the input is invalid e.g. "rgba(foo"? The invalid string "rgba(foo" will fail via Color::setNamedColor() below instead of inside the fast rgba() parser. Behavior won't change but you have a point - it will be slightly slower for invalid strings. I will update the patch. 266158 4 64719 kling 2010-08-18 09:24:52 -0700 Created attachment 64719 Proposed patch v2 Updated to avoid performance regression for invalid colors. 266206 5 64719 darin 2010-08-18 10:26:01 -0700 Comment on attachment 64719 Proposed patch v2 > +static inline bool mightBeRGBA(const UChar* characters, unsigned length) > +{ > + if (length < 5) > + return false; > + return characters[4] == '(' > + && (characters[0] == 'r' || characters[0] == 'R') > + && (characters[1] == 'g' || characters[1] == 'G') > + && (characters[2] == 'b' || characters[2] == 'B') > + && (characters[3] == 'a' || characters[3] == 'A'); > +} This kind of check can be done considerably more efficiently like this: && (characters[0] | 0x20) == 'r' && (characters[1] | 0x20) == 'b' r=me as-is but consider my faster version 266248 6 kling 2010-08-18 10:56:29 -0700 (In reply to comment #5) > r=me as-is but consider my faster version Indeed, Ariya was saying the same thing on IRC. I will land with that modification. Thanks! 266328 7 kling 2010-08-18 13:05:31 -0700 Committed r65626: <http://trac.webkit.org/changeset/65626> 64588 2010-08-17 07:13:12 -0700 2010-08-17 18:48:00 -0700 Proposed patch bug-44107.diff text/plain 2641 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NTI4OGU0ZC4uN2E5YmJiOCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMC0wOC0xNyAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIENTUzogTWFrZSByZ2IoKSBhbmQgcmdiYSgpIGZhc3QgcGF0 aHMgY2FzZS1pbnNlbnNpdGl2ZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NDQxMDcKKworICAgICAgICBBbHNvIGlubGluZWQgdGhlIHN0cmluZyBjb21w YXJpc29ucyBhZ2FpbnN0ICJyZ2IoIiBhbmQgInJnYmEoIgorICAgICAgICB3aGljaCBpcyBmYXN0 ZXIgYW5kIGF2b2lkcyBjcmVhdGluZyBhIHRlbXBvcmFyeSBTdHJpbmcgb2JqZWN0LgorCisgICAg ICAgICogY3NzL0NTU1BhcnNlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjptaWdodEJlUkdCQSk6 CisgICAgICAgIChXZWJDb3JlOjptaWdodEJlUkdCKToKKyAgICAgICAgKFdlYkNvcmU6OkNTU1Bh cnNlcjo6cGFyc2VDb2xvcik6CisKIDIwMTAtMDgtMTcgIFhhbiBMb3BleiAgPHhsb3BlekBpZ2Fs aWEuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEd1c3Rhdm8gTm9yb25oYS4KZGlmZiAtLWdp dCBhL1dlYkNvcmUvY3NzL0NTU1BhcnNlci5jcHAgYi9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3Bw CmluZGV4IGU1MTI2YTkuLjg2YjcwOTIgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvY3NzL0NTU1BhcnNl ci5jcHAKKysrIGIvV2ViQ29yZS9jc3MvQ1NTUGFyc2VyLmNwcApAQCAtMzc5Niw2ICszNzk2LDI5 IEBAIHN0YXRpYyBpbmxpbmUgYm9vbCBwYXJzZUFscGhhVmFsdWUoY29uc3QgVUNoYXIqJiBzdHJp bmcsIGNvbnN0IFVDaGFyKiBlbmQsIFVDaGFyCiAgICAgcmV0dXJuICpmb3VuZFRlcm1pbmF0b3Ig PT0gdGVybWluYXRvcjsKIH0KIAorc3RhdGljIGlubGluZSBib29sIG1pZ2h0QmVSR0JBKGNvbnN0 IFVDaGFyKiBjaGFyYWN0ZXJzLCB1bnNpZ25lZCBsZW5ndGgpCit7CisgICAgLy8gU2hvcnRlc3Qg cG9zc2libGU6ICJyZ2JhKDAsMCwwLDApIgorICAgIGlmIChsZW5ndGggPCAxMykKKyAgICAgICAg cmV0dXJuIGZhbHNlOworICAgIHJldHVybiBjaGFyYWN0ZXJzWzRdID09ICcoJworICAgICAgICAm JiAoY2hhcmFjdGVyc1swXSA9PSAncicgfHwgY2hhcmFjdGVyc1swXSA9PSAnUicpCisgICAgICAg ICYmIChjaGFyYWN0ZXJzWzFdID09ICdnJyB8fCBjaGFyYWN0ZXJzWzFdID09ICdHJykKKyAgICAg ICAgJiYgKGNoYXJhY3RlcnNbMl0gPT0gJ2InIHx8IGNoYXJhY3RlcnNbMl0gPT0gJ0InKQorICAg ICAgICAmJiAoY2hhcmFjdGVyc1szXSA9PSAnYScgfHwgY2hhcmFjdGVyc1szXSA9PSAnQScpOwor fQorCitzdGF0aWMgaW5saW5lIGJvb2wgbWlnaHRCZVJHQihjb25zdCBVQ2hhciogY2hhcmFjdGVy cywgdW5zaWduZWQgbGVuZ3RoKQoreworICAgIC8vIFNob3J0ZXN0IHBvc3NpYmxlOiAicmdiKDAs MCwwKSIKKyAgICBpZiAobGVuZ3RoIDwgMTApCisgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICBy ZXR1cm4gY2hhcmFjdGVyc1s0XSA9PSAnKCcKKyAgICAgICAgJiYgKGNoYXJhY3RlcnNbMF0gPT0g J3InIHx8IGNoYXJhY3RlcnNbMF0gPT0gJ1InKQorICAgICAgICAmJiAoY2hhcmFjdGVyc1sxXSA9 PSAnZycgfHwgY2hhcmFjdGVyc1sxXSA9PSAnRycpCisgICAgICAgICYmIChjaGFyYWN0ZXJzWzJd ID09ICdiJyB8fCBjaGFyYWN0ZXJzWzJdID09ICdCJyk7Cit9CisKIGJvb2wgQ1NTUGFyc2VyOjpw YXJzZUNvbG9yKGNvbnN0IFN0cmluZyAmbmFtZSwgUkdCQTMyJiByZ2IsIGJvb2wgc3RyaWN0KQog ewogICAgIGNvbnN0IFVDaGFyKiBjaGFyYWN0ZXJzID0gbmFtZS5jaGFyYWN0ZXJzKCk7CkBAIC0z ODEyLDcgKzM4MzUsNyBAQCBib29sIENTU1BhcnNlcjo6cGFyc2VDb2xvcihjb25zdCBTdHJpbmcg Jm5hbWUsIFJHQkEzMiYgcmdiLCBib29sIHN0cmljdCkKICAgICB9CiAKICAgICAvLyBUcnkgcmdi YSgpIHN5bnRheC4KLSAgICBpZiAobmFtZS5zdGFydHNXaXRoKCJyZ2JhKCIpKSB7CisgICAgaWYg KG1pZ2h0QmVSR0JBKGNoYXJhY3RlcnMsIGxlbmd0aCkpIHsKICAgICAgICAgY29uc3QgVUNoYXIq IGN1cnJlbnQgPSBjaGFyYWN0ZXJzICsgNTsKICAgICAgICAgY29uc3QgVUNoYXIqIGVuZCA9IGNo YXJhY3RlcnMgKyBsZW5ndGg7CiAgICAgICAgIGludCByZWQ7CkBAIC0zODM0LDcgKzM4NTcsNyBA QCBib29sIENTU1BhcnNlcjo6cGFyc2VDb2xvcihjb25zdCBTdHJpbmcgJm5hbWUsIFJHQkEzMiYg cmdiLCBib29sIHN0cmljdCkKICAgICB9CiAKICAgICAvLyBUcnkgcmdiKCkgc3ludGF4LgotICAg IGlmIChuYW1lLnN0YXJ0c1dpdGgoInJnYigiKSkgeworICAgIGlmIChtaWdodEJlUkdCKGNoYXJh Y3RlcnMsIGxlbmd0aCkpIHsKICAgICAgICAgY29uc3QgVUNoYXIqIGN1cnJlbnQgPSBjaGFyYWN0 ZXJzICsgNDsKICAgICAgICAgY29uc3QgVUNoYXIqIGVuZCA9IGNoYXJhY3RlcnMgKyBsZW5ndGg7 CiAgICAgICAgIGludCByZWQ7Cg== 64719 2010-08-18 09:24:52 -0700 2010-08-18 10:26:01 -0700 Proposed patch v2 bug-44107-v2.diff text/plain 2571 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZjlkOTQ3NC4uYmQ1YjgwOSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMC0wOC0xOCAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIENTUzogTWFrZSByZ2IoKSBhbmQgcmdiYSgpIGZhc3QgcGF0 aHMgY2FzZS1pbnNlbnNpdGl2ZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NDQxMDcKKworICAgICAgICBBbHNvIGlubGluZWQgdGhlIHN0cmluZyBjb21w YXJpc29ucyBhZ2FpbnN0ICJyZ2IoIiBhbmQgInJnYmEoIgorICAgICAgICB3aGljaCBpcyBmYXN0 ZXIgYW5kIGF2b2lkcyBjcmVhdGluZyBhIHRlbXBvcmFyeSBTdHJpbmcgb2JqZWN0LgorCisgICAg ICAgICogY3NzL0NTU1BhcnNlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjptaWdodEJlUkdCQSk6 CisgICAgICAgIChXZWJDb3JlOjptaWdodEJlUkdCKToKKyAgICAgICAgKFdlYkNvcmU6OkNTU1Bh cnNlcjo6cGFyc2VDb2xvcik6CisKIDIwMTAtMDgtMTggIFNoZXJpZmYgQm90ICA8d2Via2l0LnJl dmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHI2 NTYwMy4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvY3NzL0NTU1BhcnNlci5jcHAgYi9XZWJDb3JlL2Nz cy9DU1NQYXJzZXIuY3BwCmluZGV4IGU1MTI2YTkuLmQxNjVmNzQgMTAwNjQ0Ci0tLSBhL1dlYkNv cmUvY3NzL0NTU1BhcnNlci5jcHAKKysrIGIvV2ViQ29yZS9jc3MvQ1NTUGFyc2VyLmNwcApAQCAt Mzc5Niw2ICszNzk2LDI3IEBAIHN0YXRpYyBpbmxpbmUgYm9vbCBwYXJzZUFscGhhVmFsdWUoY29u c3QgVUNoYXIqJiBzdHJpbmcsIGNvbnN0IFVDaGFyKiBlbmQsIFVDaGFyCiAgICAgcmV0dXJuICpm b3VuZFRlcm1pbmF0b3IgPT0gdGVybWluYXRvcjsKIH0KIAorc3RhdGljIGlubGluZSBib29sIG1p Z2h0QmVSR0JBKGNvbnN0IFVDaGFyKiBjaGFyYWN0ZXJzLCB1bnNpZ25lZCBsZW5ndGgpCit7Cisg ICAgaWYgKGxlbmd0aCA8IDUpCisgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICByZXR1cm4gY2hh cmFjdGVyc1s0XSA9PSAnKCcKKyAgICAgICAgJiYgKGNoYXJhY3RlcnNbMF0gPT0gJ3InIHx8IGNo YXJhY3RlcnNbMF0gPT0gJ1InKQorICAgICAgICAmJiAoY2hhcmFjdGVyc1sxXSA9PSAnZycgfHwg Y2hhcmFjdGVyc1sxXSA9PSAnRycpCisgICAgICAgICYmIChjaGFyYWN0ZXJzWzJdID09ICdiJyB8 fCBjaGFyYWN0ZXJzWzJdID09ICdCJykKKyAgICAgICAgJiYgKGNoYXJhY3RlcnNbM10gPT0gJ2En IHx8IGNoYXJhY3RlcnNbM10gPT0gJ0EnKTsKK30KKworc3RhdGljIGlubGluZSBib29sIG1pZ2h0 QmVSR0IoY29uc3QgVUNoYXIqIGNoYXJhY3RlcnMsIHVuc2lnbmVkIGxlbmd0aCkKK3sKKyAgICBp ZiAobGVuZ3RoIDwgNCkKKyAgICAgICAgcmV0dXJuIGZhbHNlOworICAgIHJldHVybiBjaGFyYWN0 ZXJzWzNdID09ICcoJworICAgICAgICAmJiAoY2hhcmFjdGVyc1swXSA9PSAncicgfHwgY2hhcmFj dGVyc1swXSA9PSAnUicpCisgICAgICAgICYmIChjaGFyYWN0ZXJzWzFdID09ICdnJyB8fCBjaGFy YWN0ZXJzWzFdID09ICdHJykKKyAgICAgICAgJiYgKGNoYXJhY3RlcnNbMl0gPT0gJ2InIHx8IGNo YXJhY3RlcnNbMl0gPT0gJ0InKTsKK30KKwogYm9vbCBDU1NQYXJzZXI6OnBhcnNlQ29sb3IoY29u c3QgU3RyaW5nICZuYW1lLCBSR0JBMzImIHJnYiwgYm9vbCBzdHJpY3QpCiB7CiAgICAgY29uc3Qg VUNoYXIqIGNoYXJhY3RlcnMgPSBuYW1lLmNoYXJhY3RlcnMoKTsKQEAgLTM4MTIsNyArMzgzMyw3 IEBAIGJvb2wgQ1NTUGFyc2VyOjpwYXJzZUNvbG9yKGNvbnN0IFN0cmluZyAmbmFtZSwgUkdCQTMy JiByZ2IsIGJvb2wgc3RyaWN0KQogICAgIH0KIAogICAgIC8vIFRyeSByZ2JhKCkgc3ludGF4Lgot ICAgIGlmIChuYW1lLnN0YXJ0c1dpdGgoInJnYmEoIikpIHsKKyAgICBpZiAobWlnaHRCZVJHQkEo Y2hhcmFjdGVycywgbGVuZ3RoKSkgewogICAgICAgICBjb25zdCBVQ2hhciogY3VycmVudCA9IGNo YXJhY3RlcnMgKyA1OwogICAgICAgICBjb25zdCBVQ2hhciogZW5kID0gY2hhcmFjdGVycyArIGxl bmd0aDsKICAgICAgICAgaW50IHJlZDsKQEAgLTM4MzQsNyArMzg1NSw3IEBAIGJvb2wgQ1NTUGFy c2VyOjpwYXJzZUNvbG9yKGNvbnN0IFN0cmluZyAmbmFtZSwgUkdCQTMyJiByZ2IsIGJvb2wgc3Ry aWN0KQogICAgIH0KIAogICAgIC8vIFRyeSByZ2IoKSBzeW50YXguCi0gICAgaWYgKG5hbWUuc3Rh cnRzV2l0aCgicmdiKCIpKSB7CisgICAgaWYgKG1pZ2h0QmVSR0IoY2hhcmFjdGVycywgbGVuZ3Ro KSkgewogICAgICAgICBjb25zdCBVQ2hhciogY3VycmVudCA9IGNoYXJhY3RlcnMgKyA0OwogICAg ICAgICBjb25zdCBVQ2hhciogZW5kID0gY2hhcmFjdGVycyArIGxlbmd0aDsKICAgICAgICAgaW50 IHJlZDsK