I've been doing some oprofille tests with wine running fce ultra, the
8-bit Nintendo emulator. I found that when running a rom for 60
seconds, more than 99% of the CPU utilization for winex11drv (which
uses the most of all components of wine in this case) is in the same
function : convert_888_to_0888_asis (in dlls/x11drv/dib_convert.c).
I've also found that marking a couple of parameters of that function
const (that I believe should be marked const anyways), CPU usage in
that function drops measurably with oprofile. As far as I know,
parameters that aren't modified in a function should be marked const
anyways, to send the right hint to the compiler. Since it actually
turns out to be faster too, it seems worth it to me.
Here are the results from oprofile before the change:
21982 99.2012 convert_888_to_0888_asis
22078 99.1735 convert_888_to_0888_asis
22207 99.1605 convert_888_to_0888_asis
22161 99.1544 convert_888_to_0888_asis
22158 99.2253 convert_888_to_0888_asis
and after:
21828 99.1731 convert_888_to_0888_asis
21769 99.2296 convert_888_to_0888_asis
21835 99.3313 convert_888_to_0888_asis
21868 99.1027 convert_888_to_0888_asis
21601 99.1508 convert_888_to_0888_asis
On average, it went from 22117 (context switches I believe?) to 21780,
about a 1.5% improvement. The same test was run with a script each
time (run fceu with the same rom, kill it after 60 seconds). I'm
assuming the percentages (the second column in this chart) don't
matter much, because this function we're examining takes up an
overwhelming amount of the function's CPU (> 99%), so its percentage
depends largely on how many context switches other functions took up.
I've taken the liberty of providing a diff that patches all the
functions in this file to use const parameters where appropriate. I
don't get any compiler warnings with this compiling with gcc 4.0.1.
Should I submit this to wine-patches? This would be my first patch to
wine (or any open-source project, period). I'd appreciate some
feedback before I try for it.
Thanks in advance!
? patch.diff
Index: dlls/x11drv/dib_convert.c
===================================================================
RCS file: /home/wine/wine/dlls/x11drv/dib_convert.c,v
retrieving revision 1.3
diff -u -p -r1.3 dib_convert.c
--- dlls/x11drv/dib_convert.c 30 Nov 2004 21:38:58 -0000 1.3
+++ dlls/x11drv/dib_convert.c 26 Jul 2005 15:03:45 -0000
@@ -69,9 +69,9 @@
* 15 bit conversions
*/
-static void convert_5x5_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_5x5_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
int y;
@@ -83,9 +83,9 @@ static void convert_5x5_asis(int width,
}
-static void convert_555_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -115,9 +115,9 @@ static void convert_555_reverse(int widt
}
}
-static void convert_555_to_565_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_to_565_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -147,9 +147,9 @@ static void convert_555_to_565_asis(int
}
}
-static void convert_555_to_565_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_to_565_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -181,9 +181,9 @@ static void convert_555_to_565_reverse(i
}
}
-static void convert_555_to_888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_to_888_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
BYTE* dstpixel;
@@ -208,9 +208,9 @@ static void convert_555_to_888_asis(int
}
}
-static void convert_555_to_888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_to_888_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
BYTE* dstpixel;
@@ -235,9 +235,9 @@ static void convert_555_to_888_reverse(i
}
}
-static void convert_555_to_0888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_to_0888_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
DWORD* dstpixel;
@@ -261,9 +261,9 @@ static void convert_555_to_0888_asis(int
}
}
-static void convert_555_to_0888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_555_to_0888_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
DWORD* dstpixel;
@@ -287,11 +287,11 @@ static void convert_555_to_0888_reverse(
}
}
-static void convert_5x5_to_any0888(int width, int height,
- const void* srcbits, int srclinebytes,
- WORD rsrc, WORD gsrc, WORD bsrc,
- void* dstbits, int dstlinebytes,
- DWORD rdst, DWORD gdst, DWORD bdst)
+static void convert_5x5_to_any0888(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ const WORD rsrc, const WORD gsrc, const WORD bsrc,
+ void* dstbits, const int dstlinebytes,
+ const DWORD rdst, const DWORD gdst, const DWORD bdst)
{
int rRightShift1,gRightShift1,bRightShift1;
int rRightShift2,gRightShift2,bRightShift2;
@@ -352,9 +352,9 @@ static void convert_5x5_to_any0888(int w
* 16 bits conversions
*/
-static void convert_565_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -384,9 +384,9 @@ static void convert_565_reverse(int widt
}
}
-static void convert_565_to_555_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_to_555_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -414,9 +414,9 @@ static void convert_565_to_555_asis(int
}
}
-static void convert_565_to_555_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_to_555_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -446,9 +446,9 @@ static void convert_565_to_555_reverse(i
}
}
-static void convert_565_to_888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_to_888_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
BYTE* dstpixel;
@@ -473,9 +473,9 @@ static void convert_565_to_888_asis(int
}
}
-static void convert_565_to_888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_to_888_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
BYTE* dstpixel;
@@ -500,9 +500,9 @@ static void convert_565_to_888_reverse(i
}
}
-static void convert_565_to_0888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_to_0888_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
DWORD* dstpixel;
@@ -526,9 +526,9 @@ static void convert_565_to_0888_asis(int
}
}
-static void convert_565_to_0888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_565_to_0888_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const WORD* srcpixel;
DWORD* dstpixel;
@@ -557,9 +557,9 @@ static void convert_565_to_0888_reverse(
* 24 bit conversions
*/
-static void convert_888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
int y;
@@ -571,9 +571,9 @@ static void convert_888_asis(int width,
}
-static void convert_888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const BYTE* srcpixel;
BYTE* dstpixel;
@@ -594,9 +594,9 @@ static void convert_888_reverse(int widt
}
}
-static void convert_888_to_555_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_to_555_asis(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
const BYTE* srcbyte;
@@ -645,9 +645,9 @@ static void convert_888_to_555_asis(int
}
}
-static void convert_888_to_555_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_to_555_reverse(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
const BYTE* srcbyte;
@@ -696,9 +696,9 @@ static void convert_888_to_555_reverse(i
}
}
-static void convert_888_to_565_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_to_565_asis(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
const BYTE* srcbyte;
@@ -747,9 +747,9 @@ static void convert_888_to_565_asis(int
}
}
-static void convert_888_to_565_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_to_565_reverse(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
const BYTE* srcbyte;
@@ -798,9 +798,9 @@ static void convert_888_to_565_reverse(i
}
}
-static void convert_888_to_0888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_to_0888_asis(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -839,9 +839,9 @@ static void convert_888_to_0888_asis(int
}
}
-static void convert_888_to_0888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_888_to_0888_reverse(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -889,10 +889,10 @@ static void convert_888_to_0888_reverse(
}
}
-static void convert_rgb888_to_any0888(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes,
- DWORD rdst, DWORD gdst, DWORD bdst)
+static void convert_rgb888_to_any0888(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes,
+ const DWORD rdst, const DWORD gdst, const DWORD bdst)
{
int rLeftShift,gLeftShift,bLeftShift;
const BYTE* srcpixel;
@@ -916,10 +916,10 @@ static void convert_rgb888_to_any0888(in
}
}
-static void convert_bgr888_to_any0888(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes,
- DWORD rdst, DWORD gdst, DWORD bdst)
+static void convert_bgr888_to_any0888(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes,
+ const DWORD rdst, const DWORD gdst, const DWORD bdst)
{
int rLeftShift,gLeftShift,bLeftShift;
const BYTE* srcpixel;
@@ -947,9 +947,9 @@ static void convert_bgr888_to_any0888(in
* 32 bit conversions
*/
-static void convert_0888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
int y;
@@ -960,9 +960,9 @@ static void convert_0888_asis(int width,
}
}
-static void convert_0888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -983,11 +983,11 @@ static void convert_0888_reverse(int wid
}
}
-static void convert_0888_any(int width, int height,
- const void* srcbits, int srclinebytes,
- DWORD rsrc, DWORD gsrc, DWORD bsrc,
- void* dstbits, int dstlinebytes,
- DWORD rdst, DWORD gdst, DWORD bdst)
+static void convert_0888_any(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ const DWORD rsrc, const DWORD gsrc, const DWORD bsrc,
+ void* dstbits, const int dstlinebytes,
+ const DWORD rdst, const DWORD gdst, const DWORD bdst)
{
int rRightShift,gRightShift,bRightShift;
int rLeftShift,gLeftShift,bLeftShift;
@@ -1039,9 +1039,9 @@ static void convert_0888_to_555_asis(int
}
}
-static void convert_0888_to_555_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_to_555_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
WORD* dstpixel;
@@ -1062,9 +1062,9 @@ static void convert_0888_to_555_reverse(
}
}
-static void convert_0888_to_565_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_to_565_asis(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
WORD* dstpixel;
@@ -1085,9 +1085,9 @@ static void convert_0888_to_565_asis(int
}
}
-static void convert_0888_to_565_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_to_565_reverse(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
WORD* dstpixel;
@@ -1108,10 +1108,10 @@ static void convert_0888_to_565_reverse(
}
}
-static void convert_any0888_to_5x5(int width, int height,
- const void* srcbits, int srclinebytes,
- DWORD rsrc, DWORD gsrc, DWORD bsrc,
- void* dstbits, int dstlinebytes,
+static void convert_any0888_to_5x5(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ const DWORD rsrc, const DWORD gsrc, const DWORD bsrc,
+ void* dstbits, const int dstlinebytes,
WORD rdst, WORD gdst, WORD bdst)
{
int rRightShift,gRightShift,bRightShift;
@@ -1158,9 +1158,9 @@ static void convert_any0888_to_5x5(int w
}
}
-static void convert_0888_to_888_asis(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_to_888_asis(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -1197,9 +1197,9 @@ static void convert_0888_to_888_asis(int
}
}
-static void convert_0888_to_888_reverse(int width, int height,
- const void* srcbits, int srclinebytes,
- void* dstbits, int dstlinebytes)
+static void convert_0888_to_888_reverse(int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ void* dstbits, const int dstlinebytes)
{
const DWORD* srcpixel;
DWORD* dstpixel;
@@ -1250,10 +1250,10 @@ static void convert_0888_to_888_reverse(
}
}
-static void convert_any0888_to_rgb888(int width, int height,
- const void* srcbits, int srclinebytes,
- DWORD rsrc, DWORD gsrc, DWORD bsrc,
- void* dstbits, int dstlinebytes)
+static void convert_any0888_to_rgb888(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ const DWORD rsrc, const DWORD gsrc, const DWORD bsrc,
+ void* dstbits, const int dstlinebytes)
{
int rRightShift,gRightShift,bRightShift;
const DWORD* srcpixel;
@@ -1279,10 +1279,10 @@ static void convert_any0888_to_rgb888(in
}
}
-static void convert_any0888_to_bgr888(int width, int height,
- const void* srcbits, int srclinebytes,
- DWORD rsrc, DWORD gsrc, DWORD bsrc,
- void* dstbits, int dstlinebytes)
+static void convert_any0888_to_bgr888(const int width, const int height,
+ const void* srcbits, const int srclinebytes,
+ const DWORD rsrc, const DWORD gsrc, const DWORD bsrc,
+ void* dstbits, const int dstlinebytes)
{
int rRightShift,gRightShift,bRightShift;
const DWORD* srcpixel;
