239545 2022-04-20 09:08:45 -0700 Avoid call to convertToASCIILowercase() in SelectorFilter::collectElementIdentifierHashes() in the common case 2022-04-20 19:08:43 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez achristensen darin esprehn+autocc ews-watchlist ggaren glenn gyuyoung.kim macpherson menard sam webkit-bug-importer oldest_to_newest 1862952 0 cdumez 2022-04-20 09:08:45 -0700 Avoid call to convertToASCIILowercase() in SelectorFilter::collectElementIdentifierHashes() in the common case. 1862957 1 457993 cdumez 2022-04-20 09:12:51 -0700 Created attachment 457993 Patch 1863072 2 457993 sam 2022-04-20 14:42:49 -0700 Comment on attachment 457993 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457993&action=review > Source/WebCore/css/SelectorFilter.cpp:50 > + AtomString tagLowercaseLocalName = LIKELY(element.isHTMLElement() && !element.isUnknownElement()) ? element.localName() : element.localName().convertToASCIILowercase(); A comment explaining this assumption would be useful even with the assert. 1863085 3 ews-feeder 2022-04-20 15:12:34 -0700 Committed r293123 (249826@main): <https://commits.webkit.org/249826@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 457993. 1863087 4 webkit-bug-importer 2022-04-20 15:13:29 -0700 <rdar://problem/92054703> 1863144 5 457993 darin 2022-04-20 17:44:46 -0700 Comment on attachment 457993 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457993&action=review >> Source/WebCore/css/SelectorFilter.cpp:50 >> + AtomString tagLowercaseLocalName = LIKELY(element.isHTMLElement() && !element.isUnknownElement()) ? element.localName() : element.localName().convertToASCIILowercase(); > > A comment explaining this assumption would be useful even with the assert. The classic way to do this is to write a function named localNameIsKnownToBeLowercase or localNameMayContainUppercaseASCII. Then that function is the perfect place for the comment explaining why the rule is correct, and here the code is more obviously correct. 1863155 6 cdumez 2022-04-20 18:10:49 -0700 (In reply to Darin Adler from comment #5) > Comment on attachment 457993 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=457993&action=review > > >> Source/WebCore/css/SelectorFilter.cpp:50 > >> + AtomString tagLowercaseLocalName = LIKELY(element.isHTMLElement() && !element.isUnknownElement()) ? element.localName() : element.localName().convertToASCIILowercase(); > > > > A comment explaining this assumption would be useful even with the assert. > > The classic way to do this is to write a function named > localNameIsKnownToBeLowercase or localNameMayContainUppercaseASCII. Then > that function is the perfect place for the comment explaining why the rule > is correct, and here the code is more obviously correct. Oh, I missed Sam's comment before landing. Sorry about that. I'll follow-up. 1863171 7 cdumez 2022-04-20 18:52:27 -0700 0.38% progression on Speedometer on Intel, neutral on Apple Silicon. 1863174 8 cdumez 2022-04-20 19:08:43 -0700 (In reply to Darin Adler from comment #5) > Comment on attachment 457993 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=457993&action=review > > >> Source/WebCore/css/SelectorFilter.cpp:50 > >> + AtomString tagLowercaseLocalName = LIKELY(element.isHTMLElement() && !element.isUnknownElement()) ? element.localName() : element.localName().convertToASCIILowercase(); > > > > A comment explaining this assumption would be useful even with the assert. > > The classic way to do this is to write a function named > localNameIsKnownToBeLowercase or localNameMayContainUppercaseASCII. Then > that function is the perfect place for the comment explaining why the rule > is correct, and here the code is more obviously correct. Done in <https://commits.webkit.org/r293138>. 457993 2022-04-20 09:12:51 -0700 2022-04-20 15:12:35 -0700 Patch bug-239545-20220420091250.patch text/plain 2295 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjkzMDk0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZWI2Njc3YTAzNDA3NGNh YzM0MDIxNDZmNzczODMwMTY4Y2RhODNmYS4uYmQxMDdkMjkxYzY0ZWU4MGJjNTk4NzAyY2Q0MzAx MzVjMTNmMzA1ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDIyLTA0LTIwICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgQXZvaWQgY2FsbCB0byBjb252 ZXJ0VG9BU0NJSUxvd2VyY2FzZSgpIGluIFNlbGVjdG9yRmlsdGVyOjpjb2xsZWN0RWxlbWVudElk ZW50aWZpZXJIYXNoZXMoKSBpbiB0aGUgY29tbW9uIGNhc2UKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzOTU0NQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBjYWxsIHRvIGNvbnZlcnRUb0FTQ0lJTG93 ZXJjYXNlKCkgaW4gU2VsZWN0b3JGaWx0ZXI6OmNvbGxlY3RFbGVtZW50SWRlbnRpZmllckhhc2hl cygpIHNob3dzIGluIFNwZWVkb21ldGVyIHByb2ZpbGVzCisgICAgICAgIGV2ZW4gdGhvdWdoIGlu cHV0IHRhZ3MgYXJlIGFscmVhZHkgbG93ZXJjYXNlLCBzaW1wbHkgYmVjYXVzZSB0aGUgZnVuY3Rp b24gbmVlZHMgdG8gaXRlcmF0ZSBvdmVyIHRoZSBzdHJpbmcgdG8gZGV0ZXJtaW5lCisgICAgICAg IHRoYXQgaXQgaXMgYWxsIGxvd2VyY2FzZS4gSW4gdGhlIGNvbW1vbiBjYXNlIHRob3VnaCwgd2Ug Y2FuIGxldmVyYWdlIHRoZSBmYWN0IHRoYXQgYWxsIGtub3duIEhUTUwgZWxlbWVudCB0YWdzIGFy ZQorICAgICAgICBsb3dlcmNhc2UgdG8gYXZvaWQgdGhlIGNhbGwgdG8gY29udmVydFRvQVNDSUlM b3dlcmNhc2UoKS4gVGhpcyBpcyBiZWNhdXNlIEhUTUxFbGVtZW50Ojpsb2NhbE5hbWUoKSByZXR1 cm5zIG9uZSBvZiB0aGUKKyAgICAgICAgdGFncyBkZWZpbmVkIGluIEhUTUxOYW1lcy5oIChmb3Ig a25vd24gSFRNTCBlbGVtZW50cyksIGFsbCBvZiB3aGljaCBhcmUgbG93ZXJjYXNlLgorCisgICAg ICAgICogY3NzL1NlbGVjdG9yRmlsdGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNlbGVjdG9y RmlsdGVyOjpjb2xsZWN0RWxlbWVudElkZW50aWZpZXJIYXNoZXMpOgorCiAyMDIyLTA0LTIwICBT aW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29tPgogCiAgICAgICAgIFNvbWUgQXV0 b3Njcm9sbENvbnRyb2xsZXIgY2xlYW51cApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvY3Nz L1NlbGVjdG9yRmlsdGVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9TZWxlY3RvckZpbHRlci5j cHAKaW5kZXggZjhmMmNlYzFjZGMyOTQ0NjFkMmQwNTM3NzdkMzRlMmQwYWVjYTcyYi4uYjVlNGEy YmQzZWUxNTA5ZmE0NTMxNjQ4MmZhZmI4MTg1MWIzYWI0YyAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvY3NzL1NlbGVjdG9yRmlsdGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvU2Vs ZWN0b3JGaWx0ZXIuY3BwCkBAIC00Nyw3ICs0Nyw4IEBAIHN0YXRpYyBib29sIGlzRXhjbHVkZWRB dHRyaWJ1dGUoY29uc3QgQXRvbVN0cmluZyYgbmFtZSkKIAogdm9pZCBTZWxlY3RvckZpbHRlcjo6 Y29sbGVjdEVsZW1lbnRJZGVudGlmaWVySGFzaGVzKGNvbnN0IEVsZW1lbnQmIGVsZW1lbnQsIFZl Y3Rvcjx1bnNpZ25lZCwgND4mIGlkZW50aWZpZXJIYXNoZXMpCiB7Ci0gICAgQXRvbVN0cmluZyB0 YWdMb3dlcmNhc2VMb2NhbE5hbWUgPSBlbGVtZW50LmxvY2FsTmFtZSgpLmNvbnZlcnRUb0FTQ0lJ TG93ZXJjYXNlKCk7CisgICAgQXRvbVN0cmluZyB0YWdMb3dlcmNhc2VMb2NhbE5hbWUgPSBMSUtF TFkoZWxlbWVudC5pc0hUTUxFbGVtZW50KCkgJiYgIWVsZW1lbnQuaXNVbmtub3duRWxlbWVudCgp KSA/IGVsZW1lbnQubG9jYWxOYW1lKCkgOiBlbGVtZW50LmxvY2FsTmFtZSgpLmNvbnZlcnRUb0FT Q0lJTG93ZXJjYXNlKCk7CisgICAgQVNTRVJUKHRhZ0xvd2VyY2FzZUxvY2FsTmFtZSA9PSB0YWdM b3dlcmNhc2VMb2NhbE5hbWUuY29udmVydFRvQVNDSUlMb3dlcmNhc2UoKSk7CiAgICAgaWRlbnRp Zmllckhhc2hlcy5hcHBlbmQodGFnTG93ZXJjYXNlTG9jYWxOYW1lLmltcGwoKS0+ZXhpc3RpbmdI YXNoKCkgKiBUYWdOYW1lU2FsdCk7CiAKICAgICBhdXRvJiBpZCA9IGVsZW1lbnQuaWRGb3JTdHls ZVJlc29sdXRpb24oKTsK