| 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: |
|
||||||
Previously reported against Chromium as http://code.google.com/p/chromium/issues/detail?id=25252 Created attachment 41511 [details]
patch 1
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.
Patch was landed yesterday as http://trac.webkit.org/changeset/49890 |
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.)