VirtualBox

Changeset 44167 in vbox for trunk/src/VBox/Main


Ignore:
Timestamp:
Dec 19, 2012 4:40:41 PM (12 years ago)
Author:
vboxsync
Message:

Main: Increased guest property lookup performance by using a map for internal data, leaving lock before posting change event.

Location:
trunk/src/VBox/Main
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Main/include/MachineImpl.h

    r43949 r44167  
    239239         */
    240240        struct GuestProperty {
    241             /** Property name */
    242             Utf8Str strName;
    243241            /** Property value */
    244242            Utf8Str strValue;
    … …  
    291289        DragAndDropMode_T    mDragAndDropMode;
    292290
    293         typedef std::list<GuestProperty> GuestPropertyList;
    294         GuestPropertyList    mGuestProperties;
     291        typedef std::map<Utf8Str, GuestProperty> GuestPropertyMap;
     292        GuestPropertyMap     mGuestProperties;
    295293        Utf8Str              mGuestPropertyNotificationPatterns;
    296294

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

    r44152 r44167  
    16981698    AssertReturn(sizeof(HOSTCALLBACKDATA) == cbParms, VERR_INVALID_PARAMETER);
    16991699    AssertReturn(HOSTCALLBACKMAGIC == pCBData->u32Magic, VERR_INVALID_PARAMETER);
    1700     Log5(("Console::doGuestPropNotification: pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n",
    1701           pCBData->pcszName, pCBData->pcszValue, pCBData->pcszFlags));
     1700    LogFlow(("Console::doGuestPropNotification: pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n",
     1701             pCBData->pcszName, pCBData->pcszValue, pCBData->pcszFlags));
    17021702
    17031703    int  rc;
    … …  
    17141714    else
    17151715    {
    1716         LogFunc(("Console::doGuestPropNotification: hrc=%Rhrc pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n",
     1716        LogFlow(("Console::doGuestPropNotification: hrc=%Rhrc pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n",
    17171717                 hrc, pCBData->pcszName, pCBData->pcszValue, pCBData->pcszFlags));
    17181718        rc = Global::vboxStatusCodeFromCOM(hrc);

  • trunk/src/VBox/Main/src-server/MachineImpl.cpp

    r44153 r44167  
    54445444    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
    54455445    Utf8Str strName(aName);
    5446     HWData::GuestPropertyList::const_iterator it;
    5447 
    5448     for (it = mHWData->mGuestProperties.begin();
    5449          it != mHWData->mGuestProperties.end(); ++it)
    5450     {
    5451         if (it->strName == strName)
    5452         {
    5453             char szFlags[MAX_FLAGS_LEN + 1];
    5454             it->strValue.cloneTo(aValue);
    5455             *aTimestamp = it->mTimestamp;
    5456             writeFlags(it->mFlags, szFlags);
    5457             Bstr(szFlags).cloneTo(aFlags);
    5458             break;
    5459         }
    5460     }
     5446    HWData::GuestPropertyMap::const_iterator it =
     5447        mHWData->mGuestProperties.find(strName);
     5448
     5449    if (it != mHWData->mGuestProperties.end())
     5450    {
     5451        char szFlags[MAX_FLAGS_LEN + 1];
     5452        it->second.strValue.cloneTo(aValue);
     5453        *aTimestamp = it->second.mTimestamp;
     5454        writeFlags(it->second.mFlags, szFlags);
     5455        Bstr(szFlags).cloneTo(aFlags);
     5456    }
     5457
    54615458    return S_OK;
    54625459}
    … …  
    55415538    HWData::GuestProperty property;
    55425539    property.mFlags = NILFLAG;
    5543     bool found = false;
    55445540
    55455541    rc = checkStateDependency(MutableStateDep);
    … …  
    55515547        Utf8Str utf8Flags(aFlags);
    55525548        uint32_t fFlags = NILFLAG;
    5553         if (    (aFlags != NULL)
    5554              && RT_FAILURE(validateFlags(utf8Flags.c_str(), &fFlags))
     5549        if (   (aFlags != NULL)
     5550            && RT_FAILURE(validateFlags(utf8Flags.c_str(), &fFlags))
    55555551           )
    55565552            return setError(E_INVALIDARG,
    5557                             tr("Invalid flag values: '%ls'"),
     5553                            tr("Invalid guest property flag values: '%ls'"),
    55585554                            aFlags);
    55595555
    5560         /** @todo r=bird: see efficiency rant in PushGuestProperty. (Yeah, I
    5561          *                know, this is simple and do an OK job atm.) */
    5562         HWData::GuestPropertyList::iterator it;
    5563         for (it = mHWData->mGuestProperties.begin();
    5564              it != mHWData->mGuestProperties.end(); ++it)
    5565             if (it->strName == utf8Name)
     5556        HWData::GuestPropertyMap::iterator it =
     5557            mHWData->mGuestProperties.find(utf8Name);
     5558
     5559        if (it == mHWData->mGuestProperties.end())
     5560        {
     5561            setModified(IsModified_MachineData);
     5562            mHWData.backupEx();
     5563
     5564            RTTIMESPEC time;
     5565            HWData::GuestProperty prop;
     5566            prop.strValue = aValue;
     5567            prop.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time));
     5568            prop.mFlags = fFlags;
     5569
     5570            mHWData->mGuestProperties[Utf8Str(aName)] = prop;
     5571        }
     5572        else
     5573        {
     5574            if (it->second.mFlags & (RDONLYHOST))
    55665575            {
    5567                 property = *it;
    5568                 if (it->mFlags & (RDONLYHOST))
    5569                     rc = setError(E_ACCESSDENIED,
    5570                                   tr("The property '%ls' cannot be changed by the host"),
    5571                                   aName);
    5572                 else
    5573                 {
    5574                     setModified(IsModified_MachineData);
    5575                     mHWData.backup();           // @todo r=dj backup in a loop?!?
    5576 
    5577                     /* The backup() operation invalidates our iterator, so
    5578                     * get a new one. */
    5579                     for (it = mHWData->mGuestProperties.begin();
    5580                          it->strName != utf8Name;
    5581                          ++it)
    5582                         ;
    5583                     mHWData->mGuestProperties.erase(it);
    5584                 }
    5585                 found = true;
    5586                 break;
     5576                rc = setError(E_ACCESSDENIED,
     5577                              tr("The property '%ls' cannot be changed by the host"),
     5578                              aName);
    55875579            }
    5588         if (found && SUCCEEDED(rc))
    5589         {
    5590             if (aValue)
     5580            else if (aValue)
    55915581            {
     5582                setModified(IsModified_MachineData);
     5583                mHWData.backupEx();
     5584
     5585                /* The backupEx() operation invalidates our iterator,
     5586                 * so get a new one. */
     5587                it = mHWData->mGuestProperties.find(utf8Name);
     5588                Assert(it != mHWData->mGuestProperties.end());
     5589
    55925590                RTTIMESPEC time;
    5593                 property.strValue = aValue;
    5594                 property.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time));
     5591                it->second.strValue = aValue;
     5592                it->second.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time));
    55955593                if (aFlags != NULL)
    5596                     property.mFlags = fFlags;
    5597                 mHWData->mGuestProperties.push_back(property);
     5594                    it->second.mFlags = fFlags;
    55985595            }
    5599         }
    5600         else if (SUCCEEDED(rc) && aValue)
    5601         {
    5602             RTTIMESPEC time;
    5603             setModified(IsModified_MachineData);
    5604             mHWData.backup();
    5605             property.strName = aName;
    5606             property.strValue = aValue;
    5607             property.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time));
    5608             property.mFlags = fFlags;
    5609             mHWData->mGuestProperties.push_back(property);
    5610         }
     5596            else
     5597            {
     5598                setModified(IsModified_MachineData);
     5599                mHWData.backupEx();
     5600
     5601                mHWData->mGuestProperties.erase(it);
     5602            }
     5603        }
     5604
    56115605        if (   SUCCEEDED(rc)
    56125606            && (   mHWData->mGuestPropertyNotificationPatterns.isEmpty()
    … …  
    56195613           )
    56205614        {
    5621             /** @todo r=bird: Why aren't we leaving the lock here?  The
    5622              *                same code in PushGuestProperty does... */
     5615            alock.release();
     5616
    56235617            mParent->onGuestPropertyChange(mData->mUuid, aName,
    56245618                                           aValue ? aValue : Bstr("").raw(),
    … …  
    57165710    Utf8Str strPatterns(aPatterns);
    57175711
     5712    HWData::GuestPropertyMap propMap;
     5713
    57185714    /*
    57195715     * Look for matching patterns and build up a list.
    57205716     */
    5721     HWData::GuestPropertyList propList;
    5722     for (HWData::GuestPropertyList::iterator it = mHWData->mGuestProperties.begin();
    5723          it != mHWData->mGuestProperties.end();
    5724          ++it)
     5717    HWData::GuestPropertyMap::const_iterator it = mHWData->mGuestProperties.begin();
     5718    while (it != mHWData->mGuestProperties.end())
     5719    {
    57255720        if (   strPatterns.isEmpty()
    57265721            || RTStrSimplePatternMultiMatch(strPatterns.c_str(),
    57275722                                            RTSTR_MAX,
    5728                                             it->strName.c_str(),
     5723                                            it->first.c_str(),
    57295724                                            RTSTR_MAX,
    57305725                                            NULL)
    57315726           )
    5732             propList.push_back(*it);
     5727        {
     5728            propMap.insert(*it);
     5729        }
     5730
     5731        it++;
     5732    }
     5733
     5734    alock.release();
    57335735
    57345736    /*
    57355737     * And build up the arrays for returning the property information.
    57365738     */
    5737     size_t cEntries = propList.size();
     5739    size_t cEntries = propMap.size();
    57385740    SafeArray<BSTR> names(cEntries);
    57395741    SafeArray<BSTR> values(cEntries);
    … …  
    57415743    SafeArray<BSTR> flags(cEntries);
    57425744    size_t iProp = 0;
    5743     for (HWData::GuestPropertyList::iterator it = propList.begin();
    5744          it != propList.end();
    5745          ++it)
     5745
     5746    it = propMap.begin();
     5747    while (it != propMap.end())
    57465748    {
    57475749         char szFlags[MAX_FLAGS_LEN + 1];
    5748          it->strName.cloneTo(&names[iProp]);
    5749          it->strValue.cloneTo(&values[iProp]);
    5750          timestamps[iProp] = it->mTimestamp;
    5751          writeFlags(it->mFlags, szFlags);
    5752          Bstr(szFlags).cloneTo(&flags[iProp]);
    5753          ++iProp;
     5750         it->first.cloneTo(&names[iProp]);
     5751         it->second.strValue.cloneTo(&values[iProp]);
     5752         timestamps[iProp] = it->second.mTimestamp;
     5753         writeFlags(it->second.mFlags, szFlags);
     5754         Bstr(szFlags).cloneTo(&flags[iProp++]);
     5755         it++;
    57545756    }
    57555757    names.detachTo(ComSafeArrayOutArg(aNames));
    … …  
    86748676            uint32_t fFlags = guestProp::NILFLAG;
    86758677            guestProp::validateFlags(prop.strFlags.c_str(), &fFlags);
    8676             HWData::GuestProperty property = { prop.strName, prop.strValue, (LONG64) prop.timestamp, fFlags };
    8677             mHWData->mGuestProperties.push_back(property);
     8678            HWData::GuestProperty property = { prop.strValue, (LONG64) prop.timestamp, fFlags };
     8679            mHWData->mGuestProperties[prop.strName] = property;
    86788680        }
    86798681
    … …  
    98579859        data.llGuestProperties.clear();
    98589860#ifdef VBOX_WITH_GUEST_PROPS
    9859         for (HWData::GuestPropertyList::const_iterator it = mHWData->mGuestProperties.begin();
     9861        for (HWData::GuestPropertyMap::const_iterator it = mHWData->mGuestProperties.begin();
    98609862             it != mHWData->mGuestProperties.end();
    98619863             ++it)
    98629864        {
    9863             HWData::GuestProperty property = *it;
     9865            HWData::GuestProperty property = it->second;
    98649866
    98659867            /* Remove transient guest properties at shutdown unless we
    … …  
    98729874                continue;
    98739875            settings::GuestProperty prop;
    9874             prop.strName = property.strName;
     9876            prop.strName = it->first;
    98759877            prop.strValue = property.strValue;
    98769878            prop.timestamp = property.mTimestamp;
    … …  
    1274512747    com::SafeArray<BSTR> flags(cEntries);
    1274612748    unsigned i = 0;
    12747     for (HWData::GuestPropertyList::iterator it = mHWData->mGuestProperties.begin();
     12749    for (HWData::GuestPropertyMap::iterator it = mHWData->mGuestProperties.begin();
    1274812750         it != mHWData->mGuestProperties.end();
    1274912751         ++it)
    1275012752    {
    1275112753        char szFlags[MAX_FLAGS_LEN + 1];
    12752         it->strName.cloneTo(&names[i]);
    12753         it->strValue.cloneTo(&values[i]);
    12754         timestamps[i] = it->mTimestamp;
     12754        it->first.cloneTo(&names[i]);
     12755        it->second.strValue.cloneTo(&values[i]);
     12756        timestamps[i] = it->second.mTimestamp;
    1275512757        /* If it is NULL, keep it NULL. */
    12756         if (it->mFlags)
    12757         {
    12758             writeFlags(it->mFlags, szFlags);
     12758        if (it->second.mFlags)
     12759        {
     12760            writeFlags(it->second.mFlags, szFlags);
    1275912761            Bstr(szFlags).cloneTo(&flags[i]);
    1276012762        }
    … …  
    1283312835        mHWData.backup();
    1283412836
    12835         /** @todo r=bird: The careful memory handling doesn't work out here because
    12836          *  the catch block won't undo any damage we've done.  So, if push_back throws
    12837          *  bad_alloc then you've lost the value.
    12838          *
    12839          *  Another thing. Doing a linear search here isn't extremely efficient, esp.
    12840          *  since values that changes actually bubbles to the end of the list.  Using
    12841          *  something that has an efficient lookup and can tolerate a bit of updates
    12842          *  would be nice.  RTStrSpace is one suggestion (it's not perfect).  Some
    12843          *  combination of RTStrCache (for sharing names and getting uniqueness into
    12844          *  the bargain) and hash/tree is another. */
    12845         for (HWData::GuestPropertyList::iterator iter = mHWData->mGuestProperties.begin();
    12846              iter != mHWData->mGuestProperties.end();
    12847              ++iter)
    12848             if (utf8Name == iter->strName)
     12837        HWData::GuestPropertyMap::iterator it = mHWData->mGuestProperties.find(utf8Name);
     12838        if (it != mHWData->mGuestProperties.end())
     12839        {
     12840            if (aValue != NULL)
    1284912841            {
    12850                 mHWData->mGuestProperties.erase(iter);
    12851                 mData->mGuestPropertiesModified = TRUE;
    12852                 break;
     12842                it->second.strValue   = aValue;
     12843                it->second.mFlags     = fFlags;
     12844                it->second.mTimestamp = aTimestamp;
    1285312845            }
    12854         if (aValue != NULL)
    12855         {
    12856             HWData::GuestProperty property = { aName, aValue, aTimestamp, fFlags };
    12857             mHWData->mGuestProperties.push_back(property);
     12846            else
     12847                mHWData->mGuestProperties.erase(it);
     12848
    1285812849            mData->mGuestPropertiesModified = TRUE;
    1285912850        }
    … …  
    1392113912         * property store on shutdown. */
    1392213913
    13923         HWData::GuestPropertyList::iterator it;
     13914        HWData::GuestPropertyMap::const_iterator it;
    1392413915        BOOL fNeedsSaving = mData->mGuestPropertiesModified;
    1392513916        if (!fNeedsSaving)
    1392613917            for (it = mHWData->mGuestProperties.begin();
    1392713918                 it != mHWData->mGuestProperties.end(); ++it)
    13928                 if (   (it->mFlags & guestProp::TRANSIENT)
    13929                     || (it->mFlags & guestProp::TRANSRESET))
     13919                if (   (it->second.mFlags & guestProp::TRANSIENT)
     13920                    || (it->second.mFlags & guestProp::TRANSRESET))
    1393013921                {
    1393113922                    fNeedsSaving = true;
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette