[webkit-reviews] review granted: [Bug 13021] XPath can be very slow : [Attachment 13814] partial fix 3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 27 22:02:59 PDT 2007

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 13021: XPath can be very slow

Attachment 13814: partial fix 3

------- Additional Comments from Darin Adler <darin at apple.com>
+    Vector<UChar, 1024> result;
+    [...]
+    return String(result.data(), result.size());

I think it might be a slightly better idiom here is to use a plain old
Vector<UChar>, then use String::adopt for the return value. Not sure. There
will be slightly more allocation while accumulating the vector, but saves one
allocation at the resturn site.

-    return nodes;
+    return v;

I don't understand that change; can't we still return nodes?

+	     // Steals nodes from value.
+	     Value(NodeSet& value) : m_type(NodeSetValue), m_data(new
ValueData) { value.swap(m_data->m_nodeSet); }

I think maybe this is a little to dangerous to be a public constructor. Can you
instead make it a private constructor and use a named construction function to
expose this? Or maybe a clearer way would be to just construct an empty Value
and then call a function named "adopt" that takes a NodeSet&. At the very
least, I think this should be labeled "explicit".

Great changes, r=me

More information about the webkit-reviews mailing list