[webkit-reviews] review requested: [Bug 29244] undefined reference errors when linking due to gperf and inlining : [Attachment 59364] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 22 06:58:56 PDT 2010


Andras Becsi <abecsi at webkit.org> has asked  for review:
Bug 29244: undefined reference errors when linking due to gperf and inlining
https://bugs.webkit.org/show_bug.cgi?id=29244

Attachment 59364: patch
https://bugs.webkit.org/attachment.cgi?id=59364&action=review

------- Additional Comments from Andras Becsi <abecsi at webkit.org>
This patch makes the make-hash-tools.pl script generate C++ code for all gperf
files now, which are compiled in their own compilation unit, so the linking
error with gcc 4.5 in debug mode gets fixed.

Unfortunately this change introduces a potential performance drop of 0.5%-1.3%
on our page loading test suite (http://gitorious.org/methanol) with QtWebKit
trunk which has 1.5% - 3.0% deviation and thus is inconclusive and
insignificant. This drop might be due to some previously inlined functions
becoming real function calls but methanol does not convince me that the shown
drop is real because some pages show near to 1% performance increase with
0.5-1.0% deviation so this needs further investigation with other PLT suites on
other ports of WebKit (Methanol PLT uses locally mirrored popular webpages and
measures loading time using a JS framework). 

I tried several other solutions to fix the issue but none of them worked.
(Before somebody mentions it again: extern "C" doesn't fix the linking problem
because it is due to gcc 4.5's disabled inlining in debug mode and not because
of conflicting C and C++ linkage.)
 
We could work around the issue by patching the generated sources as mentioned
earlier, but this hack raises revulsive feeling in me.
If someone has a working (or partially working) solution, please contribute so
we can work out a solution which has definitely no performance penalties.

None the less I set r? for this patch because there is a memory usage dropdown
due to the smaller code size.


More information about the webkit-reviews mailing list