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 /unotools | |
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 'unotools')
-rw-r--r-- | unotools/source/config/cmdoptions.cxx | 5 | ||||
-rw-r--r-- | unotools/source/config/configitem.cxx | 6 | ||||
-rw-r--r-- | unotools/source/config/confignode.cxx | 2 | ||||
-rw-r--r-- | unotools/source/config/moduleoptions.cxx | 8 | ||||
-rw-r--r-- | unotools/source/i18n/localedatawrapper.cxx | 4 |
5 files changed, 11 insertions, 14 deletions
diff --git a/unotools/source/config/cmdoptions.cxx b/unotools/source/config/cmdoptions.cxx index 5826eb0ac082..ed1d45fb984d 100644 --- a/unotools/source/config/cmdoptions.cxx +++ b/unotools/source/config/cmdoptions.cxx @@ -281,9 +281,8 @@ Sequence< OUString > SvtCommandOptions_Impl::impl_GetPropertyNames() Sequence< OUString > lDisabledItems = GetNodeNames( SETNODE_DISABLED, utl::ConfigNameFormat::LocalPath ); // Expand all keys - std::transform(lDisabledItems.begin(), lDisabledItems.end(), lDisabledItems.begin(), - [](const OUString& rItem) -> OUString { - return SETNODE_DISABLED PATHDELIMITER + rItem + PATHDELIMITER PROPERTYNAME_CMD; }); + for (OUString& rItem : toNonConstRange(lDisabledItems)) + rItem = SETNODE_DISABLED PATHDELIMITER + rItem + PATHDELIMITER PROPERTYNAME_CMD; // Return result. return lDisabledItems; diff --git a/unotools/source/config/configitem.cxx b/unotools/source/config/configitem.cxx index 152730cec0b1..cf0384ad3651 100644 --- a/unotools/source/config/configitem.cxx +++ b/unotools/source/config/configitem.cxx @@ -316,7 +316,7 @@ Sequence< sal_Bool > ConfigItem::GetReadOnlyStates(const css::uno::Sequence< OUS // We must be sure to return a valid information every time! // Set default to non readonly... similar to the configuration handling of this property. - std::fill(lStates.begin(), lStates.end(), false); + std::fill_n(lStates.begin(), lStates.getLength(), false); // no access - no information... Reference< XHierarchicalNameAccess > xHierarchyAccess = GetTree(); @@ -655,7 +655,7 @@ static void lcl_normalizeLocalNames(Sequence< OUString >& _rNames, ConfigNameFor OUString sTypeName = xTypeContainer->getElementTemplateName(); sTypeName = sTypeName.copy(sTypeName.lastIndexOf('/')+1); - std::transform(_rNames.begin(), _rNames.end(), _rNames.begin(), + std::transform(std::cbegin(_rNames), std::cend(_rNames), _rNames.begin(), [&sTypeName](const OUString& rName) -> OUString { return wrapConfigurationElementName(rName,sTypeName); }); } else @@ -663,7 +663,7 @@ static void lcl_normalizeLocalNames(Sequence< OUString >& _rNames, ConfigNameFor Reference<XServiceInfo> xSVI(_xParentNode, UNO_QUERY); if (xSVI.is() && xSVI->supportsService("com.sun.star.configuration.SetAccess")) { - std::transform(_rNames.begin(), _rNames.end(), _rNames.begin(), + std::transform(std::cbegin(_rNames), std::cend(_rNames), _rNames.begin(), [](const OUString& rName) -> OUString { return wrapConfigurationElementName(rName); }); } } diff --git a/unotools/source/config/confignode.cxx b/unotools/source/config/confignode.cxx index f7722d6fdf25..f8bfb4c06418 100644 --- a/unotools/source/config/confignode.cxx +++ b/unotools/source/config/confignode.cxx @@ -192,7 +192,7 @@ namespace utl { aReturn = m_xDirectAccess->getElementNames(); // normalize the names - std::transform(aReturn.begin(), aReturn.end(), aReturn.begin(), + std::transform(std::cbegin(aReturn), std::cend(aReturn), aReturn.begin(), [this](const OUString& rName) -> OUString { return normalizeName(rName, NO_CONFIGURATION); }); } catch(Exception&) diff --git a/unotools/source/config/moduleoptions.cxx b/unotools/source/config/moduleoptions.cxx index a4f4c509e839..26bdb65e4638 100644 --- a/unotools/source/config/moduleoptions.cxx +++ b/unotools/source/config/moduleoptions.cxx @@ -738,16 +738,14 @@ void SvtModuleOptions_Impl::MakeReadonlyStatesAvailable() return; css::uno::Sequence< OUString > lFactories = GetNodeNames(OUString()); - std::transform(lFactories.begin(), lFactories.end(), lFactories.begin(), - [](const OUString& rFactory) -> OUString { - return rFactory + PATHSEPARATOR PROPERTYNAME_DEFAULTFILTER; - }); + for (OUString& rFactory : toNonConstRange(lFactories)) + rFactory += PATHSEPARATOR PROPERTYNAME_DEFAULTFILTER; css::uno::Sequence< sal_Bool > lReadonlyStates = GetReadOnlyStates(lFactories); sal_Int32 c = lFactories.getLength(); for (sal_Int32 i=0; i<c; ++i) { - OUString& rFactoryName = lFactories[i]; + const OUString& rFactoryName = std::as_const(lFactories)[i]; SvtModuleOptions::EFactory eFactory; if (!ClassifyFactoryByName(rFactoryName, eFactory)) diff --git a/unotools/source/i18n/localedatawrapper.cxx b/unotools/source/i18n/localedatawrapper.cxx index 9d67f1913887..782ae724c6f4 100644 --- a/unotools/source/i18n/localedatawrapper.cxx +++ b/unotools/source/i18n/localedatawrapper.cxx @@ -100,7 +100,7 @@ void LocaleDataWrapper::loadData() const css::lang::Locale& rMyLocale = maLanguageTag.getLocale(); { - Sequence< Currency2 > aCurrSeq = getAllCurrencies(); + const Sequence< Currency2 > aCurrSeq = getAllCurrencies(); if ( !aCurrSeq.hasElements() ) { if (areChecksEnabled()) @@ -133,7 +133,7 @@ void LocaleDataWrapper::loadData() { xDefaultCalendar.reset(); xSecondaryCalendar.reset(); - Sequence< Calendar2 > xCals = getAllCalendars(); + const Sequence< Calendar2 > xCals = getAllCalendars(); if (xCals.getLength() > 1) { auto pCal = std::find_if(xCals.begin(), xCals.end(), |