Bug 30579

Summary: Redundant String ref/deref calls in V8 bindings
Product: WebKit Reporter: Jens Alfke <jens>
Component: WebCore Misc.Assignee: Jens Alfke <jens>
Status: RESOLVED FIXED    
Severity: Enhancement    
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch 1 eric: review+, eric: commit-queue-

Jens Alfke
Reported 2009-10-20 10:15:49 PDT
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.)
Attachments
patch 1 (465 bytes, patch)
2009-10-20 10:17 PDT, Jens Alfke
eric: review+
eric: commit-queue-
Jens Alfke
Comment 1 2009-10-20 10:17:05 PDT
Previously reported against Chromium as http://code.google.com/p/chromium/issues/detail?id=25252
Jens Alfke
Comment 2 2009-10-20 10:17:34 PDT
Created attachment 41511 [details] patch 1
Eric Seidel (no email)
Comment 3 2009-10-20 11:47:51 PDT
Comment on attachment 41511 [details] 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.
Jens Alfke
Comment 4 2009-10-21 10:21:11 PDT
Patch was landed yesterday as http://trac.webkit.org/changeset/49890
Note You need to log in before you can comment on or make changes to this bug.