Bug 41480 – [Qt][Symbian] Variable max heap size between target/emulator

• Component: Platform
• Status: RESOLVED FIXED

[Laszlo Gombos]

Siddharth is telling me that on the simulator 96 MB for max heapsize it to high. The limit in WebKit.pri should be different for WINSCW than for target. Patches welcome.

[Siddharth Mathur]

(In reply to comment #0)
> Siddharth is telling me that on the simulator 96 MB for max heapsize it to high. The limit in WebKit.pri should be different for WINSCW than for target. Patches welcome.

How about I kill many birds with one stone, and make a QMake function that _all_  Symbian EXEs can use to set their heap sizes? Also turn on code paging and data paging automagically?

[Siddharth Mathur]

I rephrased the title to better reflect that I am doing.

[Laszlo Gombos]

Siddharth, we're already consistent I think as the HEAPSIZE is set currently for all .exe's that are built from the WebKit tree (WebKit.pri), including tests and DumpRenderTree exe's.

Also note that I do not know what are the Symbian defaults for code paging and data paging.

[Siddharth Mathur]

Created attachment 60293 [details]
Make max heap variable for ARM/emulator. Also use paging

Make max heap variable for ARM, emulator so that developers don't see jammed emulator. Also use paging. OSes that don't support the bit flag in the EXE will silently ignore it. 
Verified using QTestBrowser and other test EXEs that directives show up in MMP and ARMV5 UREL shows correct values using elf2e32

[Laszlo Gombos]

Comment on attachment 60293 [details]
Make max heap variable for ARM/emulator. Also use paging

Looks good - 2 minor issues:

1./ Can you make a reference to the bugzilla bug in the ChangeLog (prepare-ChangeLog --bug <bugid>will do that for you)? 

2./ Patch does not seems to apply on the bot and has tab character in it

[Janne Koskinen]

Hi,

PAGED keyword is deprecated and should not be used with SBSv2. Instead you should be using codepaging and datapaging keywords. These keywords also come from Qt and are added to all Qt DLLs so you don't need to add them separately.

16MB for emulator? please, that will not run anything. If you are having troubles getting more for emulator you need to tweak your environment. max heapsize defaults to 20MB in Qt which is way too low for webkit apps.
I've used 48MB in my test apps as that is the roughly the limit of my test phone and with that amount never had issues of emulator not starting but have had issues of running out of max app heap. I'd set it to 64MB and learn how to get more out of your environment.

My machine has 3GB of RAM so you might have issues if you have less than that.
Closing other programs esp. webbrowser before running emulator helps on releasing commited memory.

[Janne Koskinen]

This actually raises a question. Have you modified epoc.ini ?
I have set MegabytesOfFreeMemory 256
and test apps as 48MB...

You do realise if you set a separate limit to emulator and target you will get a very different behaviour which leads to hundreds of 'bugs' in the emulator env which can't be reproduced on HW. Or worse crashes in the emulator might hide the real bugs that only appear once we have enough memory and that is in the HW.

Users will not understand this and we end up answering a lot of forum questions and spending lot of time on debugging bogus reports. Ofc. this is issue of the emulator which we can't do much about except drop the support for good.

From my perspective the app not starting at all is much easier to explain than myriad crashes.

[Siddharth Mathur]

Created attachment 60369 [details]
increased emulator limit, and explicitly used pageddata and pagedcode directives

[Siddharth Mathur]

(In reply to comment #8)
> Created an attachment (id=60369) [details]
> increased emulator limit, and explicitly used pageddata and pagedcode directives

Janne, 
Did I address your concerns the other day over IRC? I upped the emulator limit to the higher value we discussed. Hopefully, "normal" Qt users as well as QtProd users will get more out-of-the-box usable values with this patch. 
Also, the engineers trying to get DRT to work on N8 will also see fewer crashes because the DRT binary now gets data paging by default.

[Simon Hausmann]

(In reply to comment #6)
> PAGED keyword is deprecated and should not be used with SBSv2. Instead you should be using codepaging and datapaging keywords. These keywords also come from Qt and are added to all Qt DLLs so you don't need to add them separately.

Mahesh, what about Janne's comment here?

[Siddharth Mathur]

(In reply to comment #10)
> (In reply to comment #6)
> > PAGED keyword is deprecated and should not be used with SBSv2. Instead you should be using codepaging and datapaging keywords. These keywords also come from Qt and are added to all Qt DLLs so you don't need to add them separately.
> 
> Mahesh, what about Janne's comment here?

My patch uses pageddata and pagedcode keywords, which are the more fine grained way of achieving the same effect. Janne was OK with this in my IRC chat with him, but he is on vacation it seems. 

Note that if your compiling QtWebkit/Symbian on an older toolchain say ABLD on 3.2 or 5.0, the tools will ignore the pageddata keyword with a warning printed about "unidentified keyword" . That's OK. 

Hopefully, with this patch DRT can run better on Symbian^3 due to use of data paging.

[Siddharth Mathur]

Removing paging part from bug description, as we will let Qt-Symbian's configuration handle that part.

[Siddharth Mathur]

Created attachment 63260 [details]
updated patch with paging config removed

[Siddharth Mathur]

Created attachment 63264 [details]
Correct screw up in previous patch where older TARGET.EPOCHEAPSIZE wasn't deleted

Verified that the following shows up OK in \WebKitTools\QtTestBrowser\QtTestBrowser_0xA000E543.mmp and WebKitTools\DumpRenderTree\qt\DumpRenderTree_0xE5192995.mmp (for example). 

#ifdef WINSCW
EPOCHEAPSIZE 0x40000 0x2000000 // Min 256kB, Max 32MB
#else
EPOCHEAPSIZE 0x40000 0x6000000 // Min 256kB, Max 96MB
#endif

[Laszlo Gombos]

Comment on attachment 63264 [details]
Correct screw up in previous patch where older TARGET.EPOCHEAPSIZE wasn't deleted

Looks good to me now that the explicit paging rules are removed. r+. Thanks.

[WebKit Commit Bot]

Comment on attachment 63264 [details]
Correct screw up in previous patch where older TARGET.EPOCHEAPSIZE wasn't deleted

Clearing flags on attachment: 63264

Committed r64657: <http://trac.webkit.org/changeset/64657>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Laszlo Gombos]

This patch introduced a new build warning we should look into getting rid of it..

Warning: Restricted statements detected in MMP_RULES:
         (EPOCHEAPSIZE)
         Use corresponding qmake variable(s) instead.

[Siddharth Mathur]

(In reply to comment #18)
> This patch introduced a new build warning we should look into getting rid of it..
> 
> Warning: Restricted statements detected in MMP_RULES:
>          (EPOCHEAPSIZE)
>          Use corresponding qmake variable(s) instead.

That warning is qmake's way of telling us that a normal user shouldn't have a need to use anything except TARGET.EPOCHEAPSIZE directive. Since we can't achieve conditional heap sizes with simply TARGET.EPOCHEAPSIZE, this warning is OK to incur. 
If you can verify that your generated webcore_<UID3>.mmp has the correct syntax, then things are OK.

[Simon Hausmann]

Revision r64657 cherry-picked into qtwebkit-2.1 with commit 80daa72 <http://gitorious.org/webkit/qtwebkit/commit/80daa72>

[Simon Hausmann]

re-opening as it was before the cherry-pick

[Siddharth Mathur]

I don't think any action is required on this. Hence resolving.