Use [AtomString] where appropriate in IDL files for performance. I added [AtomString] on the IDL side whenever our C++ implementation uses AtomString. Without this, the generated bindings code will generate a String, which will then get atomized once passed to our implementation. This means we're doing unnecessary String allocations in cases where the AtomString is already in the AtomStringTable.
Created attachment 457575 [details] Patch
Note that in some cases, we may be able to use [RequiresExistingAtomString] for even better performance. However, I didn't do that in this patch because this is less mechanical and requires closer inspection. I'll follow up as needed.
Comment on attachment 457575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457575&action=review Nice! > Source/WebCore/dom/Document.idl:61 > + [NewObject, ImplementedAs=createElementForBindings] Element createElement([AtomString] DOMString localName); // FIXME: missing options parameter. unrelated nit: we might want to remove this FIXME as `options` parameter is part of customised built-ins API which we won't implement. > Source/WebCore/dom/Document.idl:62 > + [NewObject] Element createElementNS([AtomString] DOMString? namespaceURI, [AtomString] DOMString qualifiedName); // FIXME: missing options parameter. ditto
Created attachment 457576 [details] Patch
Created attachment 457577 [details] Patch
Looking forward to the [RequiresExistingAtomString] follow-up too!
Comment on attachment 457577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457577&action=review > Source/WebCore/ChangeLog:9 > + Use [AtomString] where appropriate in IDL files for performance. I added [AtomString] on the > + IDL side whenever our C++ implementation uses AtomString. I remember a while back that we thought that some day we might be able to use template cleverness to get the bindings generation to do this automatically based on the type of the function and make the keyword a no-op (and then remove it). But in the mean time, and in case we never manage to figure out how to do that, it’s great to use it the way it was intended. > Source/WebCore/html/HTMLInputElement.idl:59 > + [CEReactions=NotNeeded] attribute [AtomString] DOMString type; // readonly dropped as part of DOM level 2 Wow that comment is *old*, and sure doesn't seem to add much value. But you’re so close to committing, please don’t worry about it.
Created attachment 457584 [details] Patch
Committed r292854 (249625@main): <https://commits.webkit.org/249625@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 457584 [details].
<rdar://problem/91733055>
Confirmed 0.6% progression on Speedometer on Apple Silicon (neutral on Intel) via A/B bots.