Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10140 - Simple tremor optimisations

Attached to Project: Rockbox
Opened by Dave Hooper (stripwax) - Saturday, 18 April 2009, 13:11 GMT+2
Last edited by Steve Bavin (pondlife) - Saturday, 25 April 2009, 13:43 GMT+2
Task Type Patches
Category Codecs
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Two simple optimisation patches, both applicable independently or together: one that fixes alignment checks on Coldfire, one that removes unnecessary memset on all targets.

Effect of alignment fix on coldfire (tested on h120): about 1Mhz improvement according to test_codec on 96, 350, and 500 bitrate samples; roughly unchanged at 128, 192 and 256 bitrate samples. Will have no effect on non-coldfire targets

Effect of removing redundant memsets (tested on h120): incrementally additional ~1.5Mhz improvement at 192 bitrates and above. Patch will definitely benefit other targets too (although I've not measured other targets here)
   tremor_coldfire_alignment_fix.patch (2.2 KiB) 
 apps/codecs/lib/asm_mcf5249.h       |    8 ++++----
 apps/codecs/libtremor/asm_mcf5249.h |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

   VORBIS_h120_r20728.txt (0.7 KiB)
   VORBIS_h120_r20728+alignment.txt (0.7 KiB)
   tremor_redundant_memset_removal.patch (1.1 KiB) 
 apps/codecs/libtremor/window.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

   VORBIS_h120_r20728+alignment+redundantmemset.txt (0.7 KiB)
This task depends upon

Closed by  Steve Bavin (pondlife)
Saturday, 25 April 2009, 13:43 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Closed upon request of OP.
Comment by Dave Hooper (stripwax) - Saturday, 18 April 2009, 14:14 GMT+2
On a rerun of the timings for the memset removal patch, I get much smaller improvement (which seems more realistic) - more like 0.1Mhz rather than 1.5Mhz. Rerunning a few times seems to consistently give the attached rather than the previous VORBIS_h120_r20728_alignment_redundantmemset
Benefit of the memset removal patch on its own is therefore very small on coldfire h120 - but still it really is a redundant memset, and other targets might see greater improvements in any case
   VORBIS_h120_r20728+alignment+redundantmemset-reru... (0.7 KiB)
Comment by Nils Wallménius (nls) - Saturday, 18 April 2009, 14:38 GMT+2
just a small nitpick, please use /* comments */ intead of // as per docs/CONTRIBUTING
otherwise, great to see someone improving the tremor codec! :)
Comment by Dave Hooper (stripwax) - Saturday, 18 April 2009, 15:19 GMT+2
nls - sure. memset patch updated.
   tremor_redundant_memset_removal.patch (1.1 KiB) 
 apps/codecs/libtremor/window.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comment by Dave Hooper (stripwax) - Saturday, 25 April 2009, 13:38 GMT+2
This can be closed now, as it has been incorporated into  FS#9882  and committed

Loading...