30579 2009-10-20 10:15:49 -0700 Redundant String ref/deref calls in V8 bindings 2009-10-21 10:21:11 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P3 Enhancement --- 1 jens jens oldest_to_newest 156182 0 jens 2009-10-20 10:15:49 -0700 There are unnecessary extra ref/derefs of WebCore::StringImps in the V8/WebCore bindings. This happens because the trivial inline function 'toString' defined in V8Binding.h, which is supposed to be a no-op, actually isn't: inline String toString(const String& string) { return string; } GCC generates both a String constructor and destructor call as a result of inlining this, which each generate code. This can be fixed by changing the result type to "const String&", after which the function truly is a no-op. This simple change shaves 97kbytes off of a release Mac build of Chromium, and will have some benefit in performance (though I haven't benchmarked it.) 156184 1 jens 2009-10-20 10:17:05 -0700 Previously reported against Chromium as http://code.google.com/p/chromium/issues/detail?id=25252 156185 2 41511 jens 2009-10-20 10:17:34 -0700 Created attachment 41511 patch 1 156222 3 41511 eric 2009-10-20 11:47:51 -0700 Comment on attachment 41511 patch 1 Needs a ChangeLog. Otherwise this looks fine. I trust you can add one when you commit. :) If you're not a committer, please post a new patch w/ ChangeLog. 156516 4 jens 2009-10-21 10:21:11 -0700 Patch was landed yesterday as http://trac.webkit.org/changeset/49890 41511 2009-10-20 10:17:34 -0700 2009-10-20 11:47:51 -0700 patch 1 toString.patch text/plain 465 jens SW5kZXg6IFdlYkNvcmUvYmluZGluZ3MvdjgvVjhCaW5kaW5nLmgKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2Vi Q29yZS9iaW5kaW5ncy92OC9WOEJpbmRpbmcuaAkocmV2aXNpb24gNDk3NjEpCisrKyBXZWJDb3Jl L2JpbmRpbmdzL3Y4L1Y4QmluZGluZy5oCSh3b3JraW5nIGNvcHkpCkBAIC02MSw3ICs2MSw3IEBA IG5hbWVzcGFjZSBXZWJDb3JlIHsKICAgICAgICAgcmV0dXJuIHY4Tm9uU3RyaW5nVmFsdWVUb0F0 b21pY1dlYkNvcmVTdHJpbmcodmFsdWUpOwogICAgIH0KIAotICAgIGlubGluZSBTdHJpbmcgdG9T dHJpbmcoY29uc3QgU3RyaW5nJiBzdHJpbmcpCisgICAgaW5saW5lIGNvbnN0IFN0cmluZyYgdG9T dHJpbmcoY29uc3QgU3RyaW5nJiBzdHJpbmcpCiAgICAgewogICAgICAgICByZXR1cm4gc3RyaW5n OwogICAgIH0K