Rockbox

Tasklist

FS#9886 - WPS RAM usage overhaul

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Monday, 09 February 2009, 13:30 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 07 October 2009, 07:12 GMT
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Version 3.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

The aritificial limits in the WPS have been bugging me for a while (max number of images/strings/viewports/etc) partly because there is no need for them, but also because most WPS just waste them because they dont get used.

This patch aims to replace all the static buffers used by the WPS with a single dynamically allocated buffer which both/(in the future all) WPS' use.

This isn't ready for commit yet, but it does work. This first version sets aside 2MB for the buffer which is HUGE (cabbiev2 on the e200 only uses 836 bytes).

this first version changes the progressbar, strings and images buffers only so its not quite ready.

test it out and let me know if anything breaks... also use it in the sim with the --debugwps switch and let me know how much your wps is using... end game is to (possibly) have an option to make the buffer size customisable.

also, I'm not sure if the line and token arrays should be converted... each link "wastes" an extra 4 bytes (although max tokens now is 1024 and we dont get that high often), but is crawling a linked list any slower than a regular array?
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Wednesday, 07 October 2009, 07:12 GMT
Reason for closing:  Accepted
Additional comments about closing:  in a few weeks ago
Comment by Paul Louden (Llorean) - Monday, 09 February 2009, 18:40 GMT
Doesn't setting aside 2MB significantly increase the RAM usage over what we're using now?

Shouldn't you just unify the existing buffers, but keep the RAM usage at what we already use, that way it uses the space more efficiently without actually using more?
Comment by Jonathan Gordon (jdgordon) - Monday, 09 February 2009, 23:05 GMT
like I said. this isnt ready yet.. the 2MB was just becasue I ididnt know how much to use and needed it to definetly fit... it will be much smaller in the final version
Comment by Paul Louden (Llorean) - Monday, 09 February 2009, 23:07 GMT
Ah, you didn't mention that wasn't the intended final value, so I thought it was proposed to change it to that. :) Sorry.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 10 February 2009, 07:20 GMT
thats what happens when I try to be clear at 11.30pm :p

new version moves the viewports into the buffer. an annoyingly largeish addition :p it doesnt mean anything, but just as a reference, cabbiev2 on the e200 is using 940 bytes now, (no viewports in cv2), arborboxWidgets which is a graphic/vp intensive wps i found in the wiki is up over 3k. the important thing to remeber here is these numbers are smaller by a large margin than svn's ram usage :)

there are 3 last buffers in gwps which I'm 90% sure I want to move in (the arrays for tokens, lines and sublines) and if possible the backdrop buffer (after all this frees up 200kb or something huge if the wps isnt using it). but apart from them, I'm considering commiting as is (after fixing the buffer size) to make the final version more managable... thoughts? (i'll bring this up in irc tonight probably)

side note: the wps debug text is pretty useless :p
Comment by Jonathan Gordon (jdgordon) - Tuesday, 10 February 2009, 09:00 GMT
cleanup and change the buffer size to the same as the old image buffer size (about 87K on e200)
Comment by PaulJam (PaulJam) - Tuesday, 10 February 2009, 12:25 GMT
Freezes (requiring hard reset) on H300 when trying to play music.

H300, r19963, patch v2, default settings.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 10 February 2009, 13:30 GMT
bugger :p ok than
Comment by Jonathan Gordon (jdgordon) - Sunday, 15 February 2009, 02:56 GMT
completely hard locks? or just freezes temporarily? I just tried it with one minor change on my CF H300 and it worked fine, and the minor change shouldnt affect the freezing (Without the change some legal WPS' might not load)...

I'm going to work on this a bit more today so ill opst a patch a bit later

edit: bah, the 2nd lcd screen is reiniitlaising the buffer which is Not Good (TM) :p not entirely sure what the best way to fix this is...
Comment by Jonathan Gordon (jdgordon) - Sunday, 15 February 2009, 08:21 GMT
ok, hopefully this will stop your crashing...
I have replaced the AA size display in the rockbox info screen with a display of how much RAM is being used by the WPS (to help later if/when the buffer size is customisable). This also made me realise that the numbers I posted before were nonesense because it was being displayed before the images are loaded... cabiev2 theme uses about 15KB on e200 (18KB on h300)

regarding the problem in my previous posts edit... I have fixed it on multi-lcd targets by only calling the buffer init code after a .cfg is loaded. This means that if you just load a .[r]wps manually there will be a waste in the buffer (not really a problem, but could become one if you try loading a few graphic intensive files manually) so i'm thinking that maybe we should remove the option to load these manually? btw, this is not an issue with single LCD targets because there is nothing dangerous about reinit the buffer.

like I mentioned a few posts up, there is still 3 buffers I'd like to move to this but not really sure how to do it, so maybe we can start considering this for commit as it is?
Comment by PaulJam (PaulJam) - Monday, 16 February 2009, 23:57 GMT
The v3 Patch is out of sync, so i tried it with r19998 (there was one warning during compiling: ...wps_parser.c:1523: warning: unused variable `n').

With the cabbiev2 theme (and others, for example rockboxed) the player still freezes when entering the WPS. Hard reset required.
With the rockbox_default theme playback works, but the WPS is not shown - only a blank screen with the statusbar. Rockbox stays responsive.
Another theme (which is not part of the standard build) works, but the display of images that are shown conditionally is mixed up - the first image of the conditional seems to be always shown. Rockbox stays responsive.

Player: H300
Comment by Jonathan Gordon (jdgordon) - Sunday, 01 March 2009, 21:54 GMT
resync...
I works in the sim for me but its just wierd on my h300... cabbiev2 (if loaded at boot) will play music fine and how the progress bar and info under it (except some of the icons are wrong), sometimes reloading it fixes it, sometimes it doesnt...
sort of the same with the inbuilt default

last time i tried this on my e200 it hard locked up...
Comment by Jonathan Gordon (jdgordon) - Monday, 22 June 2009, 00:21 GMT
quick resync... please give it a try and let me know how broken it is.... waiting for my ipod to charge befoe i can test
Comment by Jonathan Gordon (jdgordon) - Thursday, 13 August 2009, 05:49 GMT
OK.... time to try again...
first version... add the buffer and use it for the progressbar
Comment by Jonathan Gordon (jdgordon) - Friday, 14 August 2009, 05:09 GMT
move the images to the new buffer... assuming this works I'm thinking about commiting it as is... I dont want to get to the point I did last time where adding stuff breaks it for unknown reasons and the diff gets too big to figure out..

tested working in the sim, and clip... going to do recv1, mini2g and h300 tonight to make sure we are good
Comment by Jonathan Gordon (jdgordon) - Friday, 14 August 2009, 06:34 GMT
move the strings into this buffer also...
I tihnk this is as far as I want to take it before commitintg.... viewports is slightly messy to fix and where it blew up last time...

edit: added the error handling for when the buffer fills... testable by manually loading a .wps from the filebrowser (Which doesnt reset the buffer)

edit2: added skni_parser.c incase the patch doesnt work for you.. put it in gui/skin_engine/

Loading...