diff options
author | Mike Kaganski <mike.kaganski@collabora.com> | 2021-10-13 09:02:48 +0300 |
---|---|---|
committer | Mike Kaganski <mike.kaganski@collabora.com> | 2021-10-14 06:00:49 +0200 |
commit | 8a017d25a62e878fdd32f189f0663b05d2ffb9cf (patch) | |
tree | c91ee53b5d9276ae30df785b52579a1b77a057df /toolkit | |
parent | 17d3cacfb9675268e709cfc95771ad4ce8bde75a (diff) |
Avoid COW overhead using css::uno::Sequence
The scenarios are:
1. Calling sequence's begin() and end() in pairs to pass to algorithms
(both calls use getArray(), which does the COW checks)
2. In addition to #1, calling end() again when checking result of find
algorithms, and/or begin() to calculate result's distance
3. Using non-const sequences in range-based for loops, which internally
do #1
4. Assigning sequence to another sequence variable, and then modifying
one of them
In many cases, the sequences could be made const, or treated as const
for the purposes of the algorithms (using std::as_const, std::cbegin,
and std::cend). Where algorithm modifies the sequence, it was changed
to only call getArray() once. For that, css::uno::toNonConstRange was
introduced, which returns a struct (sublclass of std::pair) with two
iterators [begin, end], that are calculated using one call to begin()
and one call to getLength().
To handle #4, css::uno::Sequence::swap was introduced, that swaps the
internal pointer to uno_Sequence. So when a local Sequence variable
should be assigned to another variable, and the latter will be modified
further, it's now possible to use swap instead, so the two sequences
are kept independent.
The modified places were found by temporarily removing non-const end().
Change-Id: I8fe2787f200eecb70744e8b77fbdf7a49653f628
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123542
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Diffstat (limited to 'toolkit')
-rw-r--r-- | toolkit/source/controls/geometrycontrolmodel.cxx | 15 | ||||
-rw-r--r-- | toolkit/source/controls/stdtabcontroller.cxx | 8 | ||||
-rw-r--r-- | toolkit/source/controls/unocontrol.cxx | 2 | ||||
-rw-r--r-- | toolkit/source/controls/unocontrolcontainer.cxx | 6 | ||||
-rw-r--r-- | toolkit/source/controls/unocontrols.cxx | 28 | ||||
-rw-r--r-- | toolkit/source/helper/formpdfexport.cxx | 4 |
6 files changed, 32 insertions, 31 deletions
diff --git a/toolkit/source/controls/geometrycontrolmodel.cxx b/toolkit/source/controls/geometrycontrolmodel.cxx index 1a24fd417bc4..ad738a24c00c 100644 --- a/toolkit/source/controls/geometrycontrolmodel.cxx +++ b/toolkit/source/controls/geometrycontrolmodel.cxx @@ -155,8 +155,8 @@ constexpr OUStringLiteral GCM_PROPERTY_RESOURCERESOLVER = u"ResourceResolver"; sal_Int32 nOldSize = aTypes.getLength(); aTypes.realloc( nOldSize + aAggTypes.getLength() ); ::std::copy( - aAggTypes.begin(), - aAggTypes.end(), + std::cbegin(aAggTypes), + std::cend(aAggTypes), aTypes.getArray() + nOldSize ); } @@ -522,9 +522,10 @@ constexpr OUStringLiteral GCM_PROPERTY_RESOURCERESOLVER = u"ResourceResolver"; // look for duplicates, and remember them IntArrayArray::value_type& rDuplicateIds = gAmbiguousPropertyIds[ _nId ]; // for this, sort the aggregate properties + auto [begin, end] = toNonConstRange(aAggregateProps); ::std::sort( - aAggregateProps.begin(), - aAggregateProps.end(), + begin, + end, PropertyNameLess() ); @@ -532,11 +533,11 @@ constexpr OUStringLiteral GCM_PROPERTY_RESOURCERESOLVER = u"ResourceResolver"; for ( const Property& rProp : std::as_const(aProps) ) { // look for the current property in the properties of our aggregate - const Property* pAggPropPos = ::std::find_if( aAggregateProps.begin(), aAggregateProps.end(), PropertyNameEqual( rProp.Name ) ); - if ( pAggPropPos != aAggregateProps.end() ) + const Property* pAggPropPos = ::std::find_if( std::cbegin(aAggregateProps), std::cend(aAggregateProps), PropertyNameEqual( rProp.Name ) ); + if ( pAggPropPos != std::cend(aAggregateProps) ) { // found a duplicate // -> remove from the aggregate property sequence - ::comphelper::removeElementAt( aAggregateProps, pAggPropPos - aAggregateProps.begin() ); + ::comphelper::removeElementAt( aAggregateProps, pAggPropPos - std::cbegin(aAggregateProps) ); // and additionally, remember the id of this property rDuplicateIds.push_back( rProp.Handle ); diff --git a/toolkit/source/controls/stdtabcontroller.cxx b/toolkit/source/controls/stdtabcontroller.cxx index d4f5d64b04ef..1f9a3955770a 100644 --- a/toolkit/source/controls/stdtabcontroller.cxx +++ b/toolkit/source/controls/stdtabcontroller.cxx @@ -212,7 +212,7 @@ Sequence< Reference< XControl > > StdTabController::getControls( ) if ( mxControlContainer.is() ) { - Sequence< Reference< XControlModel > > aModels = mxModel->getControlModels(); + const Sequence< Reference< XControlModel > > aModels = mxModel->getControlModels(); Sequence< Reference< XControl > > xCtrls = mxControlContainer->getControls(); @@ -389,14 +389,14 @@ Reference< XControl > StdTabController::FindControl( Sequence< Reference< XCont throw lang::IllegalArgumentException("No valid XControlModel", uno::Reference<uno::XInterface>(), 0); - auto pCtrl = std::find_if(rCtrls.begin(), rCtrls.end(), + auto pCtrl = std::find_if(std::cbegin(rCtrls), std::cend(rCtrls), [&rxCtrlModel](const Reference< XControl >& rCtrl) { Reference< XControlModel > xModel(rCtrl.is() ? rCtrl->getModel() : Reference< XControlModel > ()); return xModel.get() == rxCtrlModel.get(); }); - if (pCtrl != rCtrls.end()) + if (pCtrl != std::cend(rCtrls)) { - auto n = static_cast<sal_Int32>(std::distance(rCtrls.begin(), pCtrl)); + auto n = static_cast<sal_Int32>(std::distance(std::cbegin(rCtrls), pCtrl)); Reference< XControl > xCtrl( *pCtrl ); ::comphelper::removeElementAt( rCtrls, n ); return xCtrl; diff --git a/toolkit/source/controls/unocontrol.cxx b/toolkit/source/controls/unocontrol.cxx index 0546801f2146..91b8dd756aa6 100644 --- a/toolkit/source/controls/unocontrol.cxx +++ b/toolkit/source/controls/unocontrol.cxx @@ -83,7 +83,7 @@ static Sequence< OUString> lcl_ImplGetPropertyNames( const Reference< XMultiProp DBG_ASSERT( xPSInf.is(), "UpdateFromModel: No PropertySetInfo!" ); if ( xPSInf.is() ) { - Sequence< Property> aProps = xPSInf->getProperties(); + const Sequence< Property> aProps = xPSInf->getProperties(); sal_Int32 nLen = aProps.getLength(); aNames = Sequence< OUString>( nLen ); std::transform(aProps.begin(), aProps.end(), aNames.begin(), diff --git a/toolkit/source/controls/unocontrolcontainer.cxx b/toolkit/source/controls/unocontrolcontainer.cxx index e9a146e32087..30854ba16a6f 100644 --- a/toolkit/source/controls/unocontrolcontainer.cxx +++ b/toolkit/source/controls/unocontrolcontainer.cxx @@ -700,12 +700,12 @@ void UnoControlContainer::removeTabController( const uno::Reference< awt::XTabCo { ::osl::Guard< ::osl::Mutex > aGuard( GetMutex() ); - auto pTabController = std::find_if(maTabControllers.begin(), maTabControllers.end(), + auto pTabController = std::find_if(std::cbegin(maTabControllers), std::cend(maTabControllers), [&TabController](const uno::Reference< awt::XTabController >& rTabController) { return rTabController.get() == TabController.get(); }); - if (pTabController != maTabControllers.end()) + if (pTabController != std::cend(maTabControllers)) { - auto n = static_cast<sal_Int32>(std::distance(maTabControllers.begin(), pTabController)); + auto n = static_cast<sal_Int32>(std::distance(std::cbegin(maTabControllers), pTabController)); ::comphelper::removeElementAt( maTabControllers, n ); } } diff --git a/toolkit/source/controls/unocontrols.cxx b/toolkit/source/controls/unocontrols.cxx index 0107262bf5b9..02ecedeb0389 100644 --- a/toolkit/source/controls/unocontrols.cxx +++ b/toolkit/source/controls/unocontrols.cxx @@ -2125,8 +2125,8 @@ void SAL_CALL UnoControlListBoxModel::setFastPropertyValue_NoBroadcast( sal_Int3 ::std::vector< ListItem > aItems( aStringItemList.getLength() ); ::std::transform( - aStringItemList.begin(), - aStringItemList.end(), + std::cbegin(aStringItemList), + std::cend(aStringItemList), aItems.begin(), CreateListItem() ); @@ -2605,13 +2605,13 @@ void UnoListBoxControl::addItems( const uno::Sequence< OUString>& aItems, sal_In nPos = nOldLen; // Items before the Paste-Position - std::copy(aSeq.begin(), std::next(aSeq.begin(), nPos), aNewSeq.begin()); + auto it = std::copy(std::cbegin(aSeq), std::next(std::cbegin(aSeq), nPos), aNewSeq.begin()); // New Items - std::copy(aItems.begin(), aItems.end(), std::next(aNewSeq.begin(), nPos)); + it = std::copy(aItems.begin(), aItems.end(), it); // Rest of old Items - std::copy(std::next(aSeq.begin(), nPos), aSeq.end(), std::next(aNewSeq.begin(), nPos + nNewItems)); + std::copy(std::next(std::cbegin(aSeq), nPos), std::cend(aSeq), it); ImplSetPropertyValue( GetPropertyName( BASEPROPERTY_STRINGITEMLIST ), uno::Any(aNewSeq), true ); } @@ -2633,10 +2633,10 @@ void UnoListBoxControl::removeItems( sal_Int16 nPos, sal_Int16 nCount ) uno::Sequence< OUString> aNewSeq( nNewLen ); // Items before the Remove-Position - std::copy(aSeq.begin(), std::next(aSeq.begin(), nPos), aNewSeq.begin()); + auto it = std::copy(std::cbegin(aSeq), std::next(std::cbegin(aSeq), nPos), aNewSeq.begin()); // Rest of Items - std::copy(std::next(aSeq.begin(), nPos + nCount), aSeq.end(), std::next(aNewSeq.begin(), nPos)); + std::copy(std::next(std::cbegin(aSeq), nPos + nCount), std::cend(aSeq), it); ImplSetPropertyValue( GetPropertyName( BASEPROPERTY_STRINGITEMLIST ), uno::Any(aNewSeq), true ); } @@ -2932,8 +2932,8 @@ void SAL_CALL UnoControlComboBoxModel::setFastPropertyValue_NoBroadcast( sal_Int ::std::vector< ListItem > aItems( aStringItemList.getLength() ); ::std::transform( - aStringItemList.begin(), - aStringItemList.end(), + std::cbegin(aStringItemList), + std::cend(aStringItemList), aItems.begin(), CreateListItem() ); @@ -3197,13 +3197,13 @@ void UnoComboBoxControl::addItems( const uno::Sequence< OUString>& aItems, sal_I nPos = nOldLen; // items before the insert position - std::copy(aSeq.begin(), std::next(aSeq.begin(), nPos), aNewSeq.begin()); + auto it = std::copy(std::cbegin(aSeq), std::next(std::cbegin(aSeq), nPos), aNewSeq.begin()); // New items - std::copy(aItems.begin(), aItems.end(), std::next(aNewSeq.begin(), nPos)); + it = std::copy(aItems.begin(), aItems.end(), it); // remainder of old items - std::copy(std::next(aSeq.begin(), nPos), aSeq.end(), std::next(aNewSeq.begin(), nPos + nNewItems)); + std::copy(std::next(std::cbegin(aSeq), nPos), std::cend(aSeq), it); ImplSetPropertyValue( GetPropertyName( BASEPROPERTY_STRINGITEMLIST ), Any(aNewSeq), true ); } @@ -3225,10 +3225,10 @@ void UnoComboBoxControl::removeItems( sal_Int16 nPos, sal_Int16 nCount ) uno::Sequence< OUString> aNewSeq( nNewLen ); // items before the deletion position - std::copy(aSeq.begin(), std::next(aSeq.begin(), nPos), aNewSeq.begin()); + auto it = std::copy(std::cbegin(aSeq), std::next(std::cbegin(aSeq), nPos), aNewSeq.begin()); // remainder of old items - std::copy(std::next(aSeq.begin(), nPos + nCount), aSeq.end(), std::next(aNewSeq.begin(), nPos)); + std::copy(std::next(std::cbegin(aSeq), nPos + nCount), std::cend(aSeq), it); ImplSetPropertyValue( GetPropertyName( BASEPROPERTY_STRINGITEMLIST ), uno::Any(aNewSeq), true ); } diff --git a/toolkit/source/helper/formpdfexport.cxx b/toolkit/source/helper/formpdfexport.cxx index 39c9915f77ac..9061d3ec7bce 100644 --- a/toolkit/source/helper/formpdfexport.cxx +++ b/toolkit/source/helper/formpdfexport.cxx @@ -270,7 +270,7 @@ namespace toolkitform if( ! (_rxModel->getPropertyValue( "StringItemList" ) >>= aListEntries) ) { SAL_WARN("toolkit.helper", "getStringItemVector: unable to get property StringItemList"); } - _rVector.insert( _rVector.end(), aListEntries.begin(), aListEntries.end() ); + _rVector.insert( _rVector.end(), std::cbegin(aListEntries), std::cend(aListEntries) ); } } @@ -611,7 +611,7 @@ namespace toolkitform { pListWidget->SelectedEntries.resize( 0 ); auto nEntriesSize = static_cast<sal_Int16>(pListWidget->Entries.size()); - std::copy_if(aSelectIndices.begin(), aSelectIndices.end(), std::back_inserter(pListWidget->SelectedEntries), + std::copy_if(std::cbegin(aSelectIndices), std::cend(aSelectIndices), std::back_inserter(pListWidget->SelectedEntries), [&nEntriesSize](const sal_Int16 nIndex) { return nIndex >= 0 && nIndex < nEntriesSize; }); } } |