Bug 30579 - Redundant String ref/deref calls in V8 bindings
Summary: Redundant String ref/deref calls in V8 bindings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Enhancement
Assignee: Jens Alfke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-20 10:15 PDT by Jens Alfke
Modified: 2009-10-21 10:21 PDT (History)
0 users

See Also:


Attachments
patch 1 (465 bytes, patch)
2009-10-20 10:17 PDT, Jens Alfke
eric: review+
eric: commit-queue-
Details | Formatted Diff | Diff

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