Some optimization opportunities in the V8/WebCore bindings: 1. There are redundant ref/deref calls in bindings that return objects, when the native method returns a direct pointer (e.g. Node*) but the binding code stores it temporarily in a RefPtr before returning it. 2. Since exceptions are unlikely, the calls to V8Proxy::setDOMException should be conditional (some of them weren't) and should use the UNLIKELY() macro to move the exception-handling block out of the main flow of control. In opportunity 1 we don't know if the return type of the native method is X* or PassRefPtr<X>. So to fix the problem you have to avoid generating a temporary variable at all, instead inlining the call into the return statement itself. So the binding for Node::firstChild stops doing RefPtr<Node> temp = imp->firstChild(); return V8DOMWrapper::convertNodeToV8Object(temp); and instead uses return V8DOMWrapper::convertNodeToV8Object(imp->firstChild()); This works because convertNodeToV8Object is overloaded to take either an X* or a PassRefPtr<X> as a parameter.
Created attachment 41523 [details] patch 1
Comment on attachment 41523 [details] patch 1 I don't know that I'll be able to do the final review of this, but I can give some quick feedback on a part of this. > Index: WebCore/ChangeLog > + (WebCore::V8DOMWrapper::convertToV8Object): Added overload that takes a Ref<> Answering "Why?" is much better than answering "what?" (Answering "What was done?" is pretty useless because it can be determined by looking at the code.) Functions shouldn't take a RefPtr. They should take either a PassRefPtr<T> or a T* (http://webkit.org/coding/RefPtr.html).
In general RefPtrs shouldn't be used as parameters, but this is a special case to handle generated code like the below (from V8Document.cpp): RefPtr<NodeIterator> result = imp->createNodeIterator(root, whatToShow, filter.get(), expandEntityReferences, ec); ... return V8DOMWrapper::convertToV8Object(V8ClassIndex::NODEITERATOR, result); The normal convention would be to add ".release()" after the result parameter in the second line, but the first line may or may not declare 'result' as a RefPtr and the second line doesn't know which it is. That said, I think I can add a bit of code to the binding generator to get that .release() call in there...
Created attachment 41605 [details] patch 2 This updated patch avoids creating a new method that takes a Ref<>. As a bonus, it now changes only one source file.
Comment on attachment 41605 [details] patch 2 r=me.
Comment on attachment 41605 [details] patch 2 Clearing flags on attachment: 41605 Committed r49996: <http://trac.webkit.org/changeset/49996>
All reviewed patches have been landed. Closing bug.