Changeset 98095 in vbox

Timestamp:

Jan 16, 2023 3:17:49 PM (23 months ago)

Author:

vboxsync

Message:

Main/Console: Implemented a LED query optimization, where also keep track of LEDs on a per-type. This will prevent scanning all the sets and also skip any DeviceType_Null entries. bugref:9892

Location:

trunk/src/VBox/Main

Files:

• include/ConsoleImpl.h (modified) (3 diffs)
• src-client/ConsoleImpl.cpp (modified) (8 diffs)
• src-client/ConsoleImpl2.cpp (modified) (5 diffs)

trunk/src/VBox/Main/include/ConsoleImpl.h

@@ -810 +810 @@
     typedef struct LEDSET *PLEDSET;
     PPDMLED volatile *i_getLedSet(uint32_t iLedSet);
+    void i_setLedType(DeviceType_T *penmSubTypeEntry, DeviceType_T enmNewType);
+    HRESULT i_refreshLedTypeArrays(AutoReadLock *pReadLock);
     uint32_t i_allocateDriverLeds(uint32_t cLeds, uint32_t fTypes, DeviceType_T **ppSubTypes);
-    void i_attachStatusDriver(PCFGMNODE pCtlInst, DeviceType_T enmType);
+    void i_attachStatusDriver(PCFGMNODE pCtlInst, DeviceType_T enmType, uint32_t cLeds = 1);
     void i_attachStatusDriver(PCFGMNODE pCtlInst, uint32_t fTypes, uint32_t cLeds, DeviceType_T **ppaSubTypes,
                               Console::MediumAttachmentMap *pmapMediumAttachments,
@@ -1068 +1070 @@
     /** @name LEDs and their management
      * @{ */
-    /** Read/write lock separating LED allocations (write) from queries (read). */
+    /** Read/write lock separating LED allocations and per-type data construction
+     * (write) from queries (read). */
     RWLockHandle            mLedLock;
+    /** LED configuration generation.  This is increased whenever a new set is
+     *  allocated or a sub-device type changes. */
+    uint32_t                muLedGen;
+    /** The LED configuration generation which maLedTypes was constructed for. */
+    uint32_t                muLedTypeGen;
     /** Number of LED sets in use in maLedSets. */
     uint32_t                mcLedSets;
@@ -1085 +1093 @@
         DeviceType_T       *paSubTypes;
     } maLedSets[32];
+    /** LEDs data organized by DeviceType_T.
+     * This is reconstructed by Console::i_refreshLedTypeArrays() when
+     * Console::getDeviceActivity is called and mLedTypeGen doesn't match
+     * muLedGen. */
+    struct
+    {
+        /** Number of possibly valid entries in pappLeds. */
+        uint32_t            cLeds;
+        /** Number of allocated entries. */
+        uint32_t            cAllocated;
+        /** Array of pointer to LEDSET::papLed entries.
+         * The indirection is due to Console::i_drvStatus_UnitChanged() only knowing
+         * about the LEDSET::papLeds. */
+        PPDMLED volatile  **pappLeds;
+    } maLedTypes[DeviceType_End];
     /** @} */
 

trunk/src/VBox/Main/src-client/ConsoleImpl.cpp

@@ -428 +428 @@
     , mBusMgr(NULL)
     , mLedLock(LOCKCLASS_LISTOFOTHEROBJECTS /* must be higher than LOCKCLASS_OTHEROBJECT */)
+    , muLedGen(0)
+    , muLedTypeGen(0)
+    , mcLedSets(0)
     , m_pKeyStore(NULL)
     , mpIfSecKey(NULL)
@@ -438 +441 @@
     , mpCryptoIf(NULL)
 {
+    RT_ZERO(maLedSets);
+    RT_ZERO(maLedTypes);
 }
 
@@ -446 +451 @@
 {
     LogFlowThisFunc(("\n"));
-
-    mcLedSets = 0;
-    RT_ZERO(maLedSets);
 
     MYVMM2USERMETHODS *pVmm2UserMethods = (MYVMM2USERMETHODS *)RTMemAllocZ(sizeof(*mpVmm2UserMethods) + sizeof(Console *));
@@ -881 +883 @@
 
     /* Release memory held by the LED sets (no need to take lock). */
+    for (size_t idxType = 0; idxType < RT_ELEMENTS(maLedTypes); idxType++)
+    {
+        maLedTypes[idxType].cLeds = 0;
+        maLedTypes[idxType].cAllocated = 0;
+        RTMemFree(maLedTypes[idxType].pappLeds);
+        maLedTypes[idxType].pappLeds = NULL;
+    }
     for (size_t idxSet = 0; idxSet < mcLedSets; idxSet++)
     {
+        maLedSets[idxSet].cLeds = 0;
         RTMemFree((void *)maLedSets[idxSet].papLeds);
         maLedSets[idxSet].papLeds = NULL;
@@ -2815 +2825 @@
 }
 
+/**
+ * Refreshes the maLedTypes and muLedTypeGen members.
+ */
+HRESULT Console::i_refreshLedTypeArrays(AutoReadLock *pReadLock)
+{
+    pReadLock->release();
+    AutoWriteLock alock(mLedLock COMMA_LOCKVAL_SRC_POS);
+
+    /*
+     * Check that the refresh was already done by someone else while we
+     * acquired the write lock.
+     */
+    if (muLedTypeGen != muLedGen)
+    {
+        /*
+         * Reset the data.
+         */
+        for (size_t idxType = 0; idxType < RT_ELEMENTS(maLedTypes); idxType++)
+            maLedTypes[idxType].cLeds = 0;
+
+        /*
+         * Rebuild the data.
+         */
+        for (uint32_t idxSet = 0; idxSet < mcLedSets; idxSet++)
+        {
+            PLEDSET const            pLS        = &maLedSets[idxSet];
+            uint32_t const           cLeds      = pLS->cLeds;
+            PPDMLED volatile * const papSrcLeds = pLS->papLeds;
+            DeviceType_T * const     paSubTypes = pLS->paSubTypes;
+            for (uint32_t idxLed = 0; idxLed < cLeds; idxLed++)
+            {
+                /** @todo If we make Console::i_drvStatus_UnitChanged() modify the generation
+                 * too, we could skip NULL entries here and make it a bit more compact.
+                 * OTOH, most unused LED entires have a paSubTypes of DeviceType_Null. */
+                DeviceType_T enmType = paSubTypes ? paSubTypes[idxLed] : (DeviceType_T)(ASMBitFirstSetU32(pLS->fTypes) - 1);
+                if (enmType > DeviceType_Null && enmType < DeviceType_End)
+                {
+                    uint32_t const idxLedType = maLedTypes[enmType].cLeds;
+                    if (idxLedType >= maLedTypes[enmType].cAllocated)
+                    {
+                        void *pvNew = RTMemRealloc(maLedTypes[enmType].pappLeds,
+                                                   sizeof(maLedTypes[0].pappLeds[0]) * (idxLedType + 16));
+                        if (!pvNew)
+                            return E_OUTOFMEMORY;
+                        maLedTypes[enmType].pappLeds   = (PPDMLED volatile  **)pvNew;
+                        maLedTypes[enmType].cAllocated = idxLedType + 16;
+                    }
+                    maLedTypes[enmType].pappLeds[idxLedType] = &papSrcLeds[idxLed];
+                    maLedTypes[enmType].cLeds                = idxLedType + 1;
+                }
+            }
+        }
+        muLedTypeGen = muLedGen;
+    }
+
+    /*
+     * We have to release the write lock before re-acquiring the read-lock.
+     *
+     * This means there is a theoretical race here, however we ASSUME that
+     * LED sets are never removed and therefore we will be just fine
+     * accessing slightly dated per-type data.
+     */
+    alock.release();
+    pReadLock->acquire();
+    return S_OK;
+}
+
 /** read the value of a LED. */
 DECLINLINE(uint32_t) readAndClearLed(PPDMLED pLed)
@@ -2828 +2905 @@
 {
     /*
-     * Make a roadmap of which DeviceType_T LED types are wanted:
+     * Make a roadmap of which DeviceType_T LED types are wanted.
+     *
+     * Note! This approach means we'll return the same values in aActivity for
+     *       duplicate aType entries.
      */
-    uint32_t fWanted = 0;
+    uint32_t fRequestedTypes = 0;
     AssertCompile(DeviceType_End <= 32);
 
@@ -2836 +2916 @@
     {
         DeviceType_T const enmType = aType[iType];
+        AssertCompile((unsigned)DeviceType_Null == 0 /* first */);
         AssertReturn(enmType > DeviceType_Null && enmType < DeviceType_End,
                      setError(E_INVALIDARG, tr("Invalid DeviceType for getDeviceActivity in entry #%u: %d"), iType, enmType));
-        fWanted |= RT_BIT_32((unsigned)enmType);
-    }
-
-    /* Resize the result vector before making changes (may throw, paranoia). */
+        fRequestedTypes |= RT_BIT_32((unsigned)enmType);
+    }
+
+    /*
+     * Resize the result vector before making changes (may throw, paranoia).
+     */
     aActivity.resize(aType.size());
 
     /*
-     * Collect all the LEDs in a single sweep through all drivers' sets:
-     *
-     * Because this method can be called by the frontend and others while the
-     * VM is being constructed, we use a dedicated lock to prevent stumbling
-     * into the allocator.
+     * Accumulate the per-type data for all the requested types.
+     * We will lazily refresh the per-type data collection here when needed.
      */
     PDMLEDCORE aLEDs[DeviceType_End] = { {0} };
@@ -2855 +2935 @@
     {
         AutoReadLock alock(mLedLock COMMA_LOCKVAL_SRC_POS);
-        uint32_t idxSet = mcLedSets;
-        while (idxSet-- > 0)
-        {
-            /* Look inside this driver's set of LEDs and check if the types mask overlap with the request: */
-            PLEDSET pLS = &maLedSets[idxSet];
-            if (pLS->fTypes & fWanted)
+        if (RT_LIKELY(muLedGen == muLedTypeGen))
+        { /* likely */ }
+        else
+        {
+            HRESULT hrc = i_refreshLedTypeArrays(&alock);
+            if (FAILED(hrc))
+                return hrc;
+        }
+
+        AssertCompile((unsigned)DeviceType_Null == 0 /* we skip this one */);
+        for (uint32_t idxType = 1; idxType < RT_ELEMENTS(maLedTypes); idxType++)
+            if (fRequestedTypes & RT_BIT_32(idxType))
             {
-                uint32_t const           cLeds      = pLS->cLeds;
-                PPDMLED volatile * const papSrcLeds = pLS->papLeds;
-
-                /* Multi-type drivers (e.g. SCSI) have a subtype array which must be matched. */
-                DeviceType_T const *paSubTypes = pLS->paSubTypes;
-                if (paSubTypes)
-                    for (uint32_t idxLed = 0; idxLed < cLeds; idxLed++)
-                    {
-                        DeviceType_T const enmType = paSubTypes[idxLed];
-                        Assert((unsigned)enmType < (unsigned)DeviceType_End);
-                        if (fWanted & RT_BIT_32((unsigned)enmType))
-                            aLEDs[enmType].u32 |= readAndClearLed(papSrcLeds[idxLed]);
-                    }
-                /* Single-type drivers (e.g. floppy) have the type in ->enmType */
-                else
-                {
-                    uint32_t const idxType = ASMBitFirstSetU32(pLS->fTypes) - 1;
-                    for (uint32_t idxLed = 0; idxLed < cLeds; idxLed++)
-                        aLEDs[idxType].u32 |= readAndClearLed(papSrcLeds[idxLed]);
-                }
+                uint32_t const            cLeds       = maLedTypes[idxType].cLeds;
+                PPDMLED volatile ** const pappSrcLeds = maLedTypes[idxType].pappLeds;
+                for (size_t iLed = 0; iLed < cLeds; iLed++)
+                    aLEDs[idxType].u32 |= readAndClearLed(*pappSrcLeds[iLed]);
             }
-        }
     }
 

trunk/src/VBox/Main/src-client/ConsoleImpl2.cpp

@@ -657 +657 @@
 
 /**
+ * Updates the device type for a LED.
+ *
+ * @param   penmSubTypeEntry    The sub-type entry to update.
+ * @param   enmNewType          The new type.
+ */
+void Console::i_setLedType(DeviceType_T *penmSubTypeEntry, DeviceType_T enmNewType)
+{
+    /*
+     * ASSUMES no race conditions here wrt concurrent type updating.
+     */
+    if (*penmSubTypeEntry != enmNewType)
+    {
+        *penmSubTypeEntry = enmNewType;
+        ASMAtomicIncU32(&muLedGen);
+    }
+}
+
+
+/**
  * Allocate a set of LEDs.
  *
@@ -738 +757 @@
  * @throws ConfigError and std::bad_alloc.
  */
-void Console::i_attachStatusDriver(PCFGMNODE pCtlInst, DeviceType_T enmType)
+void Console::i_attachStatusDriver(PCFGMNODE pCtlInst, DeviceType_T enmType, uint32_t cLeds /*= 1*/)
 {
     Assert(enmType > DeviceType_Null && enmType < DeviceType_End);
-    i_attachStatusDriver(pCtlInst, RT_BIT_32(enmType), 1, NULL, NULL, NULL, 0);
+    i_attachStatusDriver(pCtlInst, RT_BIT_32(enmType), cLeds, NULL, NULL, NULL, 0);
 }
 
@@ -2184 +2203 @@
                      * Attach the status driver.
                      */
-                    i_attachStatusDriver(pInst, RT_BIT_32(DeviceType_USB), 2, NULL, NULL, NULL, 0);
+                    i_attachStatusDriver(pInst, DeviceType_USB, 2);
                 }
             } /* for every USB controller. */
@@ -2485 +2504 @@
 
                     /* Attach the status driver */
-                    i_attachStatusDriver(pCtlInst, DeviceType_Floppy, 2, NULL,
-                                         &mapMediumAttachments, pszCtrlDev, ulInstance);
+                    i_attachStatusDriver(pCtlInst, RT_BIT_32(DeviceType_Floppy),
+                                         2, NULL, &mapMediumAttachments, pszCtrlDev, ulInstance);
                     break;
                 }
@@ -5097 +5116 @@
 
         if (paLedDevType)
-            paLedDevType[uLUN] = enmType;
+            i_setLedType(&paLedDevType[uLUN], enmType);
 
         /* Dump the changed LUN if possible, dump the complete device otherwise */