All the bindings for interfaces that extend Node have the following: v8::Handle<v8::Object> V8HTMLAnchorElement::wrapSlow(HTMLAnchorElement* impl) { v8::Handle<v8::Object> wrapper; V8Proxy* proxy = 0; if (impl->document()) { proxy = V8Proxy::retrieve(impl->document()->frame()); if (proxy && static_cast<Node*>(impl->document()) == static_cast<Node*>(impl)) { if (proxy->windowShell()->context().IsEmpty() && proxy->windowShell()->initContextIfNeeded()) { // initContextIfNeeded may have created a wrapper for the object, retry from the start. return V8HTMLAnchorElement::wrap(impl); } } } ... The important line here is if (proxy && static_cast<Node*>(impl->document()) == static_cast<Node*>(impl)) { which seems to be only true if impl is a document. I believe we can remove this check in the code generator.
We can also remove if (impl->document()) { for all nodes except DocumentType nodes.
Created attachment 129727 [details] Patch
Adam, Kentaro, Nate? Can anyone review this?
Comment on attachment 129727 [details] Patch Can you run run-bindings-tests --reset-results so we can see the change in the generated code?
Comment on attachment 129727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129727&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3038 > + if (IsNodeSubType($dataNode) && $interfaceName ne "DocumentType") { > + $proxyInit = "V8Proxy::retrieve(impl->document()->frame())"; > + } else { > + $proxyInit = "0"; Why is $proxyInit always zero when $interfaceName ne "DocumentType" ? I would have expected the DocumentType case to need a null-check on impl->document(). Presumably when impl->document() is non-null, the "DocumentType" case should work the same as the other nodes.
Do you have any stats on how much performance we gain with this change? If so, you might want to put them in the ChangeLog.
Comment on attachment 129727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129727&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3038 >> + $proxyInit = "0"; > > Why is $proxyInit always zero when $interfaceName ne "DocumentType" ? I would have expected the DocumentType case to need a null-check on impl->document(). Presumably when impl->document() is non-null, the "DocumentType" case should work the same as the other nodes. I misunderstood your earlier comment. I thought you said that the document() of a DocumentType i always null. However it might be null so a check is needed for that. I will update this.
(In reply to comment #6) > Do you have any stats on how much performance we gain with this change? If so, you might want to put them in the ChangeLog. I haven't. The perf tests takes too long to run. I was just going to monitor the perf bots instead.
Oh, sorry. The DocumentType node can have a null document (but also can have a non-null document). Other types of nodes always have non-null documents.
Sorry about forgetting about run-binding-tests. However, none of the test bindings are Nodes so this will not be visible in there (The added UNLIKELY will show up though).
Created attachment 129930 [details] Patch
Comment on attachment 129930 [details] Patch Clearing flags on attachment: 129930 Committed r109611: <http://trac.webkit.org/changeset/109611>
All reviewed patches have been landed. Closing bug.