Bug 50270 - [Qt] [Symbian] Reintroduce compiler optimizations for JSC
Summary: [Qt] [Symbian] Reintroduce compiler optimizations for JSC
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC S60 3rd edition
: P2 Normal
Assignee: Norbert Leser
URL:
Keywords: Performance, Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-11-30 13:36 PST by Laszlo Gombos
Modified: 2012-05-21 07:18 PDT (History)
8 users (show)

See Also:


Attachments
Patch that adds optimization to JavaScriptCore lib (1.36 KB, patch)
2010-11-30 14:05 PST, Norbert Leser
laszlo.gombos: review-
Details | Formatted Diff | Diff
Patch that removes superfluous compiler optimization from WebCore (935 bytes, patch)
2010-11-30 14:33 PST, Norbert Leser
no flags Details | Formatted Diff | Diff
Updated patch for WebCore with corrected Changelog styling (1.00 KB, patch)
2010-11-30 14:42 PST, Norbert Leser
no flags Details | Formatted Diff | Diff
Updated patch for JavaScriptCore with corrected Changelog styling (1.40 KB, patch)
2010-11-30 14:45 PST, Norbert Leser
no flags Details | Formatted Diff | Diff
remove optimization + ALWAYS_ARM for WebCore (1.25 KB, patch)
2010-12-15 17:54 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2010-11-30 13:36:22 PST
Commit r53315 (http://trac.webkit.org/changeset/53315) significantly improved the performance of JavaScript execution by adding time-based optimization (-Otime) and -O3 to symbian builds.

Commit r56623 (http://trac.webkit.org/changeset/56623) split up building JSC and rest of Webkit and us such we lost the optimization for JSC. This seems to be a significant performance regression between WebKit 2.0 and WebKit 2.1 releases. 

Perhaps we no longer need the optimizations for WebCore now that we can only turn it on for JSC. QtScript should greatly benefit from this work as well.
Comment 1 Norbert Leser 2010-11-30 14:05:48 PST
Created attachment 75201 [details]
Patch that adds optimization to JavaScriptCore lib

The included patch adds armcc compiler flags for symbian builds to optimize JavaScriptCore library for performance (-O3 and -OTime).
Also, the performance improving MMP rule ALWAYS_BUILD_AS_ARM is added conditionally for symbian builds on windows (linux builds use THUMB).

This patch was verified against JS performance tests Sunspider and V8. In both cases, the optimization flag increases performance significantly (35% and 16%, respectively).
Comment 2 Laszlo Gombos 2010-11-30 14:30:34 PST
Comment on attachment 75201 [details]
Patch that adds optimization to JavaScriptCore lib

View in context: https://bugs.webkit.org/attachment.cgi?id=75201&action=review

> JavaScriptCore/ChangeLog:8
> +

ChangeLog should follow the style - see http://webkit.org/coding/contributing.html#changelogs.

It should look like:
[Qt] [Symbian] Reintroduce compiler optimizations for JSC
https://bugs.webkit.org/show_bug.cgi?id=50270

Add compiler optimization (symbian ARM target) which was lost after split from WebCore.
Tested via Sunspider and V8 - both of which show significant performance improvement.


Otherwise patch looks good to me, but I would like to see more discussion on if we can remove the optimizations from WebCore.
Comment 3 Norbert Leser 2010-11-30 14:33:49 PST
Created attachment 75206 [details]
Patch that removes superfluous compiler optimization from WebCore

This patch removes the -O3 -OTime compiler optimization for Symbian ARM release targets. This one was initially introduced when JavaScriptCore was embedded in WebCore's build but is not really necessary for WebCore. Sunspider and V8 tests show no performance differences whatsoever.

However, removing this optimization reduces the runtime code size by 33%. Counting the slight code size increase due to optimization in JavaScriptCore only (6%), overall we'll still have 27% less code with a total of (uncompressed) 9.8MB.
Comment 4 Norbert Leser 2010-11-30 14:42:57 PST
Created attachment 75208 [details]
Updated patch for WebCore with corrected Changelog styling

Corrected Changelog styling
Comment 5 Norbert Leser 2010-11-30 14:45:52 PST
Created attachment 75209 [details]
Updated patch for JavaScriptCore with corrected Changelog styling

Corrected Changelog styling
Comment 6 Suresh Voruganti 2010-11-30 16:35:10 PST
Ademar, pls cherry pick the fix for Qtwebkit 2.1. This is blocker for Qtwebkit 2.1 release criteria.
Comment 7 Laszlo Gombos 2010-11-30 21:25:17 PST
Comment on attachment 75209 [details]
Updated patch for JavaScriptCore with corrected Changelog styling

r+, thanks.
Comment 8 WebKit Commit Bot 2010-12-02 05:58:28 PST
Comment on attachment 75209 [details]
Updated patch for JavaScriptCore with corrected Changelog styling

Clearing flags on attachment: 75209

Committed r73126: <http://trac.webkit.org/changeset/73126>
Comment 9 Laszlo Gombos 2010-12-02 08:02:34 PST
(In reply to comment #3)
> Created an attachment (id=75206) [details]
> Patch that removes superfluous compiler optimization from WebCore
> 
> This patch removes the -O3 -OTime compiler optimization for Symbian ARM release targets. This one was initially introduced when JavaScriptCore was embedded in WebCore's build but is not really necessary for WebCore. Sunspider and V8 tests show no performance differences whatsoever.
> 
> However, removing this optimization reduces the runtime code size by 33%. Counting the slight code size increase due to optimization in JavaScriptCore only (6%), overall we'll still have 27% less code with a total of (uncompressed) 9.8MB.

I'm in favor of this change. I also would like to give a chance to Simon and Janne to voice their opinion (CCd).

Perhaps we should look into if we still need the following section in WebCore.pro 


    symbian-abld|symbian-sbsv2 {
        # RO text (code) section in qtwebkit.dll exceeds allocated space for gcce udeb target.
        # Move RW-section base address to start from 0xE00000 instead of the toolchain default 0x400000.
        QMAKE_LFLAGS.ARMCC += --rw-base 0xE00000
        MMP_RULES += ALWAYS_BUILD_AS_ARM
    }  else {
        QMAKE_CFLAGS -= --thumb
        QMAKE_CXXFLAGS -= --thumb
    }
Comment 10 Ademar Reis 2010-12-02 13:16:07 PST
Revision r73126 cherry-picked into qtwebkit-2.1 with commit e7c8392 <http://gitorious.org/webkit/qtwebkit/commit/e7c8392>
Comment 11 Janne Koskinen 2010-12-03 00:10:40 PST
(In reply to comment #9)
> Perhaps we should look into if we still need the following section in WebCore.pro 
> 
> 
>     symbian-abld|symbian-sbsv2 {
>         # RO text (code) section in qtwebkit.dll exceeds allocated space for gcce udeb target.
>         # Move RW-section base address to start from 0xE00000 instead of the toolchain default 0x400000.
>         QMAKE_LFLAGS.ARMCC += --rw-base 0xE00000
>         MMP_RULES += ALWAYS_BUILD_AS_ARM
>     }  else {
>         QMAKE_CFLAGS -= --thumb
>         QMAKE_CXXFLAGS -= --thumb
>     }


If you can build webcore as thumb then sure. Reason why it was added there was that thumb jump range was exceeded after adding -O3 and -OTime . For GCCE I think it would have to stay as GCCE generates twice the .code size than RVCT.

BTW: We had late discovery that vfpu instructions need ARM mode so it might be good idea to try enabling them for jscore now with this insight in mind :)
Comment 12 Laszlo Gombos 2010-12-15 17:54:27 PST
Created attachment 76719 [details]
remove optimization + ALWAYS_ARM for WebCore
Comment 13 Caio Marcelo de Oliveira Filho 2012-05-21 07:18:59 PDT
Closing as WONTFIX because we do not develop QtWebKit for Symbian anymore.