[Qemu-devel] [Patch] Clear memory using memset instead of handcoded loop

[Qemu-devel] [Patch] Clear memory using memset instead of handcoded loop

[Daniel Egger]

[-- Attachment #1.1: Type: text/plain, Size: 547 bytes --]

Hija,

while profiling the lastest qemu snapshot on OS X I discovered
that one of the tb_hash clearing routines turnes up to be a
"hotspot" during my typical test procedure which includes
booting an installed Debian Sarge RC.

Attached patch turnes three handcoded memory clearing routines
into a call to memset which will be optimized by every serious
compiler into either a very optimized inline sequence for the
CPU or at least into a call of an optimized memory clearing
function (bzero on Darwin 7.5 with gcc 3.3).

Please CC me on replies.


[-- Attachment #1.2: diff --]
[-- Type: application/octet-stream, Size: 1255 bytes --]

diff -ur qemu-snapshot-2004-10-01_23/exec.c qemu-snapshot-2004-10-01_23.my/exec.c
--- qemu-snapshot-2004-10-01_23/exec.c	Fri Oct  1 00:22:08 2004
+++ qemu-snapshot-2004-10-01_23.my/exec.c	Sun Oct  3 12:39:17 2004
@@ -315,7 +316,6 @@
 /* XXX: tb_flush is currently not thread safe */
 void tb_flush(CPUState *env)
 {
-    int i;
 #if defined(DEBUG_FLUSH)
     printf("qemu: flush code_size=%d nb_tbs=%d avg_tb_size=%d\n", 
            code_gen_ptr - code_gen_buffer, 
@@ -323,12 +323,10 @@
            nb_tbs > 0 ? (code_gen_ptr - code_gen_buffer) / nb_tbs : 0);
 #endif
     nb_tbs = 0;
-    for(i = 0;i < CODE_GEN_HASH_SIZE; i++)
-        tb_hash[i] = NULL;
+    memset (tb_hash, 0, CODE_GEN_HASH_SIZE * sizeof (void *));
     virt_page_flush();
 
-    for(i = 0;i < CODE_GEN_PHYS_HASH_SIZE; i++)
-        tb_phys_hash[i] = NULL;
+    memset (tb_phys_hash, 0, CODE_GEN_HASH_SIZE * sizeof (void *));
     page_flush_tb();
 
     code_gen_ptr = code_gen_buffer;
@@ -1279,8 +1277,7 @@
     }
 
     virt_page_flush();
-    for(i = 0;i < CODE_GEN_HASH_SIZE; i++)
-        tb_hash[i] = NULL;
+    memset (tb_hash, 0, CODE_GEN_HASH_SIZE * sizeof (void *));
 
 #if !defined(CONFIG_SOFTMMU)
     munmap((void *)MMAP_AREA_START, MMAP_AREA_END - MMAP_AREA_START);

[-- Attachment #1.3: Type: text/plain, Size: 23 bytes --]



Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [Patch] Clear memory using memset instead of handcoded loop

[Fabrice Bellard]

Applied.

Fabrice.

Daniel Egger wrote:
> Hija,
> 
> while profiling the lastest qemu snapshot on OS X I discovered
> that one of the tb_hash clearing routines turnes up to be a
> "hotspot" during my typical test procedure which includes
> booting an installed Debian Sarge RC.
> 
> Attached patch turnes three handcoded memory clearing routines
> into a call to memset which will be optimized by every serious
> compiler into either a very optimized inline sequence for the
> CPU or at least into a call of an optimized memory clearing
> function (bzero on Darwin 7.5 with gcc 3.3).
> 
> Please CC me on replies.
> 
> 
> 
> Servus,
>       Daniel
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

Re: [Qemu-devel] [Patch] Clear memory using memset instead of handcoded loop

[Karl Magdsick]

Daniel, 

Good catch!  

Is the section still a hot spot in your tests?  Maybe a macro or
inline function would be more appropriate.  The macro/inline function
could be defined to use memset for now, and later changed to use
optimized inline assembly language on architectures that don't inline
memset.  It is also likely possible to write a slightly faster inline
assembly routine since we know that we want to always set the memory
to zero, while memset has to allow for an arbitrary fill value.


-Karl

On Sun, 3 Oct 2004 13:45:51 +0200, Daniel Egger <degger@fhm.edu> wrote:
> Hija,
> 
> while profiling the lastest qemu snapshot on OS X I discovered
> that one of the tb_hash clearing routines turnes up to be a
> "hotspot" during my typical test procedure which includes
> booting an installed Debian Sarge RC.
> 
> Attached patch turnes three handcoded memory clearing routines
> into a call to memset which will be optimized by every serious
> compiler into either a very optimized inline sequence for the
> CPU or at least into a call of an optimized memory clearing
> function (bzero on Darwin 7.5 with gcc 3.3).
> 
> Please CC me on replies.
> 
> 
> 
> 
> Servus,
>        Daniel
> 
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
> 
> 
> 
> 
>

Re: [Qemu-devel] [Patch] Clear memory using memset instead of handcoded loop

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 1884 bytes --]

On 03.10.2004, at 20:17, Karl Magdsick wrote:

> Is the section still a hot spot in your tests?

A short test after the change revealed that this indeed freed a
few cpu cycles which have been spent elsewhere. But since I've now
a bzero call in the profiles this function because another hotspot.

So it is an improvement but the most optimal would be to reduce the
amount of calls to memset or the containing function.

> Maybe a macro or
> inline function would be more appropriate.  The macro/inline function
> could be defined to use memset for now, and later changed to use
> optimized inline assembly language on architectures that don't inline
> memset.  It is also likely possible to write a slightly faster inline
> assembly routine since we know that we want to always set the memory
> to zero, while memset has to allow for an arbitrary fill value.

It's very unlikely that a compiler will not optimize memset,
especially when the parameters are constant, since this is in the
C standard since C89 and used everywhere. I very much believe that
it is close to impossible to produce a better handcoded version
neither in assembly nor in C: The provided memset is usually the
optimum for the platform even when used non-inline, so duplicating
it does not make sense. Handcoding it in ASM will be always
suboptimal since the compiler will always schedule it as is
without taking the surrounding code into account which is typically
a huge loss compared to a C version.

Since memset is a builtin in GCC the compiler can (and will!) use
the additional information provided by an explicit call of the
builtin for further optimisation where possible. Actually the
current gcc (3.4) is still pretty stupid about this particular
optimisation but since this is low-hanging (compiler) fruit the
new tree optimizers in gcc 4.x will optimize the heck out of that.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]