Re: Bochs icache perf when writing memory

Hi,

I applied your patch with small modifications and saw nice speedup over my
WinXP boot benchmark.

Some comments to your patch (might be you could improve it following the
comments):

v2h_write_byte is necessary  to invalidate the whole page in ICache because
it is used in fast string operations (REP MOVS, REP STOS and other similar
x86 instructions).
For example see faststring.cc: Bit32u BX_CPU_C::FastRepSTOSD(bx_address
laddrDst, Bit32u val, Bit32u count)
The function "saves" calling decWriteStamp on every copied 4-byte block and
instead invalidates entire page first.
Assuming that if you are doing memset/memcpy on same page, you probably
should not have code in the same 4K page.
It is certainly correct for modern OSes but might be not so correct for old
stuff like WinXP. I don't know what is better -> calling decWriteStamp many
times precisely or once but with 4K granularity.
Can you advice ?

Thanks,
Stanislav

-----Original Message-----
From: Lander Brandt <[email protected]> 
Sent: Sunday, 2 August 2020 5:01
To: [email protected]
Subject: [Bochs-developers] Bochs icache perf when writing memory

Hello,

I was recently doing some profiling of Bochs and noticed a lot of time in my
build spent in the instruction decoder caused by icache misses. After
reading the code intimately for a few days I *think* I have a pretty decent
understanding of how it works and may have fixed some performance issues,
but I would like to present my changes and see if what I changed is "legal".

In memory.cc I noticed that I was hitting the memcpy-type case in the
BX_MEM_C::{read,write}PhysicalPage methods and in every case I documented it
was a read/write of 16 bytes. For writes this seemed particularly bad
because it would call `pageWriteStampTable.decWriteStamp(a20addr);`, which
if my understanding of the icache is correct may cause an entire page to be
flushed from the icache for even small writes. I "fixed" this (it still has
unnecessary work when the length is not a multiple of 8) and a similar
condition in `BX_CPU_C::v2h_write_byte`.

Anecdotally I noticed a very good performance increase by just eyeballing
the IPS counter in the bottom-left of the GUI and the number of calls to
`bxICache_c::handleSMC` with an 0xFFFF_FFFF mask dropped from 16,125,122 for
~1BN instructions to just 34. Again if my understanding of the icache is
correct (and please explain it to me if my understanding is fundamentally
broken), this resulted in a substantially fewer whole pages being flushed
from the icache if they were previously mapped in. I also did not observe
any *visible* negative impacts of this patch and my snapshot appeared to
operate normally.

Thanks!

Here is my patch:

diff --git a/bochs_src/cpu/access.cc b/bochs_src/cpu/access.cc index
da1148ad..4a72f5ce 100644
--- a/bochs_src/cpu/access.cc
+++ b/bochs_src/cpu/access.cc
@@ -513,7 +513,7 @@ BX_CPU_C::v2h_write_byte(bx_address laddr, bx_bool user)
       bx_hostpageaddr_t hostPageAddr = tlbEntry->hostPageAddr;
       Bit32u pageOffset = PAGE_OFFSET(laddr);
       Bit8u *hostAddr = (Bit8u*) (hostPageAddr | pageOffset);
-      pageWriteStampTable.decWriteStamp(tlbEntry->ppf);
+      pageWriteStampTable.decWriteStamp(tlbEntry->ppf, 1);
       return hostAddr;
     }
   }
diff --git a/bochs_src/memory/memory.cc b/bochs_src/memory/memory.cc index
e896f1d1..d3fa737c 100644
--- a/bochs_src/memory/memory.cc
+++ b/bochs_src/memory/memory.cc
@@ -110,8 +110,6 @@ mem_write:
       // len == other, just fall thru to special cases handling
     }
 
-    pageWriteStampTable.decWriteStamp(a20addr);
-
 #ifdef BX_LITTLE_ENDIAN
     data_ptr = (Bit8u *) data;
 #else // BX_BIG_ENDIAN
@@ -122,18 +120,38 @@ mem_write:
     {
       // addr *not* in range 000A0000 .. 000FFFFF
       while(1) {
-        *(BX_MEM_THIS get_vector(a20addr)) = *data_ptr;
-        if (len == 1) return;
-        len--;
-        a20addr++;
-#ifdef BX_LITTLE_ENDIAN
-        data_ptr++;
-#else // BX_BIG_ENDIAN
-        data_ptr--;
-#endif
+        // Read in chunks of 8 bytes if we can
+        if (len % 8 == 0) {
+          pageWriteStampTable.decWriteStamp(a20addr, 8);
+          WriteHostQWordToLittleEndian(BX_MEM_THIS get_vector(a20addr),
*(Bit64u*)data_ptr);
+          len -= 8;
+          a20addr += 8;
+          #ifdef BX_LITTLE_ENDIAN
+            data_ptr += 8;
+          #else
+            data_ptr -= 8;
+          #endif
+
+          if (len == 0) return;
+        } else {
+          pageWriteStampTable.decWriteStamp(a20addr, 1);
+          *(BX_MEM_THIS get_vector(a20addr)) = *data_ptr;
+          if (len == 1) return;
+          len--;
+          a20addr++;
+  #ifdef BX_LITTLE_ENDIAN
+          data_ptr++;
+  #else // BX_BIG_ENDIAN
+          data_ptr--;
+  #endif
+        }
       }
     }
 
+
+    printf("LENGTH NOT HANDLED!!! %d\n", len);
+    pageWriteStampTable.decWriteStamp(a20addr);
+
     // addr must be in range 000A0000 .. 000FFFFF
 
     for(unsigned i=0; i<len; i++) {
@@ -273,15 +291,29 @@ mem_read:
     {
       // addr *not* in range 000A0000 .. 000FFFFF
       while(1) {
-        *data_ptr = *(BX_MEM_THIS get_vector(a20addr));
-        if (len == 1) return;
-        len--;
-        a20addr++;
-#ifdef BX_LITTLE_ENDIAN
-        data_ptr++;
-#else // BX_BIG_ENDIAN
-        data_ptr--;
-#endif
+        // Read in chunks of 8 bytes if we can
+        if (len % 8 == 0) {
+          ReadHostQWordFromLittleEndian(BX_MEM_THIS get_vector(a20addr),
*(Bit64u*)data_ptr);
+          len -= 8;
+          a20addr += 8;
+          #ifdef BX_LITTLE_ENDIAN
+            data_ptr += 8;
+          #else
+            data_ptr -= 8;
+          #endif
+
+          if (len == 0) return;
+        } else {
+          *data_ptr = *(BX_MEM_THIS get_vector(a20addr));
+          if (len == 1) return;
+          len--;
+          a20addr++;
+  #ifdef BX_LITTLE_ENDIAN
+          data_ptr++;
+  #else // BX_BIG_ENDIAN
+          data_ptr--;
+  #endif
+        }
       }
     }
 


_______________________________________________
bochs-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bochs-developers