Bug 21500 - Qt-port on Linux should be built with g++ -O3 switch
Summary: Qt-port on Linux should be built with g++ -O3 switch
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-09 02:58 PDT by Csaba Osztrogonác
Modified: 2009-04-16 18:22 PDT (History)
2 users (show)

See Also:


Attachments
proposed patch (949 bytes, patch)
2008-10-09 05:44 PDT, Csaba Osztrogonác
timothy: review+
Details | Formatted Diff | Diff
proposed patch (1.11 KB, patch)
2008-10-27 03:35 PDT, Csaba Osztrogonác
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2008-10-09 02:58:18 PDT
WebKit Qt-port on Linux is built with g++ -O2 switch because of Qt default config (Qt-4.4.1/mkspecs/common/g++.conf) Building with -O3 switch instead of -O2, JSC is faster. (I used gcc version 4.1.2, Qt version 4.4.1 and WebKit rev37424)

runtime and code size of JSC:
- 3.51% speedup on SunSpider (+/-0.8%)
- 1.30% speedup on V8 (+/-0.65%)
- 3.81% speedup on WindScorpion (+/-1.0%)
- Code size of JSC .text section increased by 9.96%

I'll send patch soon.
Comment 1 Csaba Osztrogonác 2008-10-09 05:11:36 PDT
Layout tests result - the whole WebKit built with -O3:
489.63s total testing time
7272 test cases (89%) succeeded
836 test cases (10%) had incorrect layout
3 test cases (<1%) crashed

Layout tests result - only JSC built with -O3:
513.98s total testing time
7277 test cases (89%) succeeded
831 test cases (10%) had incorrect layout
3 test cases (<1%) crashed

Layout tests result - original 37424 revision (with default -O2): 
531.12s total testing time
7272 test cases (89%) succeeded
836 test cases (10%) had incorrect layout
3 test cases (<1%) crashed
Comment 2 Csaba Osztrogonác 2008-10-09 05:44:13 PDT
Created attachment 24230 [details]
proposed patch
Comment 3 Alp Toker 2008-10-09 09:25:30 PDT
(In reply to comment #2)
> Created an attachment (id=24230) [edit]
> proposed patch
> 

It's incorrect to build WebCore with -O3 I believe (only JavaScriptCore should be -03).
Comment 4 Csaba Osztrogonác 2008-10-10 00:50:06 PDT
(In reply to comment #3)
> It's incorrect to build WebCore with -O3 I believe (only JavaScriptCore should
> be -03).

Why is it incorrect? Does building WebCore with -O3 cause any problem? It seems to work fine (and faster) here with -O3. Maybe on other platforms have a problem with -O3. A little code size growth can be bad for embedded systems, but not for PC's.
Comment 5 Simon Hausmann 2008-10-21 05:52:47 PDT
I agree with Alp here, only JavaScriptCore should be build with O3. Can you patch JavaScriptCore/JavaScriptCore.pri instead?
Comment 6 Csaba Osztrogonác 2008-10-27 02:23:31 PDT
(In reply to comment #5)
I will create patch for ToT and measure its speedup soon.
Comment 7 Csaba Osztrogonác 2008-10-27 03:32:57 PDT
I've created new patch for JavaScriptCore/JavaScriptCore.pri at r37894.

runtime and code size of JSC:
- 2.02% speedup on SunSpider (+/-0.8%)
- 1.10% speedup on V8 (+/-0.7%)
- 3.45% speedup on WindScorpion (+/-1.1%)
- Code size of JSC .text section increased by 9.70%
Comment 8 Csaba Osztrogonác 2008-10-27 03:35:52 PDT
Created attachment 24686 [details]
proposed patch
Comment 9 Csaba Osztrogonác 2008-11-13 05:52:47 PST
new results for r38367:
- 1.41% speedup on SunSpider (+/-0.5%)
- 0.12% speedup on V8 (+/-0.3%) - not significant
- 6.33% speedup on WindScorpion (+/-0.3%)
- Code size of JSC .text section increased by 9.70%

There isn't activity a long time. Is anybody interested in landing this patch?
Comment 10 Simon Hausmann 2008-11-14 10:50:31 PST
Comment on attachment 24686 [details]
proposed patch

Looks good. I'll also add -fstrict-aliasing when landing like we do in WebCore/WebCore.pro, so that we are on the safe side.
Comment 11 Simon Hausmann 2008-11-14 10:53:18 PST
Nevermind the comment about aliasing, that was nonsense :-)

Landed your patch in r38398. Thanks!