Bug 50270 – [Qt] [Symbian] Reintroduce compiler optimizations for JSC

• Component: JavaScriptCore
• Status: RESOLVED WONTFIX

[Laszlo Gombos]

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.

[Norbert Leser]

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).

[Laszlo Gombos]

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.

[Norbert Leser]

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.

[Norbert Leser]

Created attachment 75208 [details]
Updated patch for WebCore with corrected Changelog styling

Corrected Changelog styling

[Norbert Leser]

Created attachment 75209 [details]
Updated patch for JavaScriptCore with corrected Changelog styling

Corrected Changelog styling

[Suresh Voruganti]

Ademar, pls cherry pick the fix for Qtwebkit 2.1. This is blocker for Qtwebkit 2.1 release criteria.

[Laszlo Gombos]

Comment on attachment 75209 [details]
Updated patch for JavaScriptCore with corrected Changelog styling

r+, thanks.

[WebKit Commit Bot]

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>

[Laszlo Gombos]

(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
    }

[Ademar Reis]

Revision r73126 cherry-picked into qtwebkit-2.1 with commit e7c8392 <http://gitorious.org/webkit/qtwebkit/commit/e7c8392>

[Janne Koskinen]

(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 :)

[Laszlo Gombos]

Created attachment 76719 [details]
remove optimization + ALWAYS_ARM for WebCore

[Caio Marcelo de Oliveira Filho]

Closing as WONTFIX because we do not develop QtWebKit for Symbian anymore.