Changeset 33278 in vbox for trunk/src/VBox/Runtime/common

Timestamp:

Oct 20, 2010 9:36:48 PM (14 years ago)

Author:

vboxsync

Message:

RTMemCache: Fixed a bug in a calculation affect (too) small object and/or alignments. Tried to fix a more serious issue in the free list handling, but failed - disabled the free list optimization instead.

File:

• trunk/src/VBox/Runtime/common/alloc/memcache.cpp (modified) (7 diffs)

trunk/src/VBox/Runtime/common/alloc/memcache.cpp

@@ -163 +163 @@
     if (cbAlignment == 0)
     {
-        cbAlignment = (size_t)1 << ASMBitLastSetU32((uint32_t)cbObject);
-        if (cbAlignment > 64)
+        if (cbObject <= 2)
+            cbAlignment = cbObject;
+        else if (cbObject <= 4)
+            cbAlignment = 4;
+        else if (cbObject <= 8)
+            cbAlignment = 8;
+        else if (cbObject <= 16)
+            cbAlignment = 16;
+        else if (cbObject <= 32)
+            cbAlignment = 32;
+        else
             cbAlignment = 64;
     }
@@ -190 +199 @@
     pThis->cbAlignment      = (uint32_t)cbAlignment;
     pThis->cPerPage         = (uint32_t)((PAGE_SIZE - RT_ALIGN_Z(sizeof(RTMEMCACHEPAGE), cbAlignment)) / pThis->cbObject);
-    while (    RT_ALIGN_Z(sizeof(RTMEMCACHEPAGE), RT_MIN(cbAlignment, 8))
-             + pThis->cPerPage * pThis->cbObject
-             + RT_ALIGN(pThis->cPerPage, 64) / 8 * 2
+    while (  RT_ALIGN_Z(sizeof(RTMEMCACHEPAGE), 8)
+           + pThis->cPerPage * pThis->cbObject
+           + RT_ALIGN(pThis->cPerPage, 64) / 8 * 2
            > PAGE_SIZE)
         pThis->cPerPage--;
@@ -209 +218 @@
     pThis->pFreeTop         = NULL;
 
+    /** @todo
+     * Here is a puzzler (or maybe I'm just blind), the free list code breaks
+     * badly on my macbook pro (i7) (32-bit).
+     *
+     * I tried changing the reads from unordered to ordered to no avail.  Then I
+     * tried optimizing the code with the ASMAtomicCmpXchgExPtr function to
+     * avoid some reads - no change. Inserting pause instructions did nothing
+     * (as expected).  The only thing which seems to make a difference is
+     * reading the pFreeTop pointer twice in the the free code... This is weird
+     * or I'm overlooking something..
+     *
+     * No time to figure it out, so I'm disabling the broken code paths for
+     * now. */
+    pThis->fUseFreeList = false;
+
     *phMemCache = pThis;
     return VINF_SUCCESS;
@@ -223 +247 @@
 #ifdef RT_STRICT
     uint32_t cFree = pThis->cFree;
-    for (PRTMEMCACHEFREEOBJ pFree = pThis->pFreeTop; pFree; pFree = pFree->pNext)
+    for (PRTMEMCACHEFREEOBJ pFree = pThis->pFreeTop; pFree && cFree < pThis->cTotal + 5; pFree = pFree->pNext)
         cFree++;
     AssertMsg(cFree == pThis->cTotal, ("cFree=%u cTotal=%u\n", cFree, pThis->cTotal));
@@ -295 +319 @@
             pb = RT_ALIGN_PT(pb, 8, uint8_t *);
             pPage->pbmCtor      = pb;
-            pb += pThis->cBits / 8 * 2;
             pb = (uint8_t *)pPage + PAGE_SIZE - pThis->cbObject * cObjects;
             pPage->pbObjects    = pb;   Assert(RT_ALIGN_P(pb, pThis->cbAlignment) == pb);
@@ -362 +385 @@
     if (pObj)
     {
-        PRTMEMCACHEFREEOBJ pNext;
         do
         {
-            pNext = ASMAtomicUoReadPtrT(&pObj->pNext, PRTMEMCACHEFREEOBJ);
-            if (ASMAtomicCmpXchgPtr(&pThis->pFreeTop, pNext, pObj))
+            PRTMEMCACHEFREEOBJ pNext = ASMAtomicUoReadPtrT(&pObj->pNext, PRTMEMCACHEFREEOBJ);
+            PRTMEMCACHEFREEOBJ pObjOld;
+            if (ASMAtomicCmpXchgExPtr(&pThis->pFreeTop, pNext, pObj, &pObjOld))
             {
+                Assert(pObjOld == pObj);
+                Assert(pNext != pObjOld);
+                pObj->pNext = NULL;
                 *ppvObj = pObj;
                 return VINF_SUCCESS;
             }
-            pObj = ASMAtomicUoReadPtrT(&pThis->pFreeTop, PRTMEMCACHEFREEOBJ);
+            pObj = pObjOld;
+            ASMNopPause();
         } while (pObj);
     }
@@ -508 +535 @@
          */
         PRTMEMCACHEFREEOBJ pObj = (PRTMEMCACHEFREEOBJ)pvObj;
-        PRTMEMCACHEFREEOBJ pNext;
-        do
-        {
-            pNext = ASMAtomicUoReadPtrT(&pThis->pFreeTop, PRTMEMCACHEFREEOBJ);
+        PRTMEMCACHEFREEOBJ pNext = ASMAtomicUoReadPtrT(&pThis->pFreeTop, PRTMEMCACHEFREEOBJ);
+        PRTMEMCACHEFREEOBJ pFreeTopOld;
+        pObj->pNext = pNext;
+        while (!ASMAtomicCmpXchgExPtr(&pThis->pFreeTop, pObj, pNext, &pFreeTopOld))
+        {
+            pNext = pFreeTopOld;
+            Assert(pNext != pObj);
             pObj->pNext = pNext;
-        } while (!ASMAtomicCmpXchgPtr(&pThis->pFreeTop, pObj, pNext));
+            ASMNopPause();
+        }
     }
     else