From 8a017d25a62e878fdd32f189f0663b05d2ffb9cf Mon Sep 17 00:00:00 2001
From: Mike Kaganski <mike.kaganski@collabora.com>
Date: Wed, 13 Oct 2021 09:02:48 +0300
Subject: 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>
---
 i18npool/qa/cppunit/test_ordinalsuffix.cxx         | 38 +++++++---------------
 .../ignoreIandEfollowedByYa_ja_JP.cxx              |  3 +-
 .../transliteration/ignoreIterationMark_ja_JP.cxx  |  3 +-
 .../ignoreKiKuFollowedBySa_ja_JP.cxx               |  3 +-
 .../ignoreProlongedSoundMark_ja_JP.cxx             |  3 +-
 .../source/transliteration/transliterationImpl.cxx | 34 ++++++++-----------
 .../transliteration/transliteration_OneToOne.cxx   |  3 +-
 .../transliteration/transliteration_body.cxx       |  5 +--
 8 files changed, 38 insertions(+), 54 deletions(-)

(limited to 'i18npool')

diff --git a/i18npool/qa/cppunit/test_ordinalsuffix.cxx b/i18npool/qa/cppunit/test_ordinalsuffix.cxx
index fb06a41fa4b2..be21f38ca531 100644
--- a/i18npool/qa/cppunit/test_ordinalsuffix.cxx
+++ b/i18npool/qa/cppunit/test_ordinalsuffix.cxx
@@ -46,50 +46,36 @@ void TestOrdinalSuffix::tearDown()
 void TestOrdinalSuffix::testFrench()
 {
     lang::Locale aLocale("fr", "LU", "");
-    uno::Sequence< OUString > aSuffixes;
-    OUString *pStart, *pEnd, *pFind;
 
     //1er
-    aSuffixes = m_xOrdinal->getOrdinalSuffix(1, aLocale);
-    pStart = aSuffixes.begin();
-    pEnd = aSuffixes.end();
-    pFind = std::find(pStart, pEnd, OUString("er"));
-    CPPUNIT_ASSERT(pFind != pEnd);
+    uno::Sequence<OUString> aSuffixes = m_xOrdinal->getOrdinalSuffix(1, aLocale);
+    const OUString* pFind = std::find(std::cbegin(aSuffixes), std::cend(aSuffixes), OUString("er"));
+    CPPUNIT_ASSERT(pFind != std::cend(aSuffixes));
 
     //2e, 3e, etc.
     aSuffixes = m_xOrdinal->getOrdinalSuffix(2, aLocale);
-    pStart = aSuffixes.begin();
-    pEnd = aSuffixes.end();
-    pFind = std::find(pStart, pEnd, OUString("e"));
-    CPPUNIT_ASSERT(pFind != pEnd);
+    pFind = std::find(std::cbegin(aSuffixes), std::cend(aSuffixes), OUString("e"));
+    CPPUNIT_ASSERT(pFind != std::cend(aSuffixes));
 }
 
 void TestOrdinalSuffix::testEnglish()
 {
     lang::Locale aLocale("en", "US", "");
-    uno::Sequence< OUString > aSuffixes;
-    OUString *pStart, *pEnd, *pFind;
 
     //1st
-    aSuffixes = m_xOrdinal->getOrdinalSuffix(1, aLocale);
-    pStart = aSuffixes.begin();
-    pEnd = aSuffixes.end();
-    pFind = std::find(pStart, pEnd, OUString("st"));
-    CPPUNIT_ASSERT(pFind != pEnd);
+    uno::Sequence<OUString> aSuffixes = m_xOrdinal->getOrdinalSuffix(1, aLocale);
+    const OUString* pFind = std::find(std::cbegin(aSuffixes), std::cend(aSuffixes), OUString("st"));
+    CPPUNIT_ASSERT(pFind != std::cend(aSuffixes));
 
     //2nd
     aSuffixes = m_xOrdinal->getOrdinalSuffix(2, aLocale);
-    pStart = aSuffixes.begin();
-    pEnd = aSuffixes.end();
-    pFind = std::find(pStart, pEnd, OUString("nd"));
-    CPPUNIT_ASSERT(pFind != pEnd);
+    pFind = std::find(std::cbegin(aSuffixes), std::cend(aSuffixes), OUString("nd"));
+    CPPUNIT_ASSERT(pFind != std::cend(aSuffixes));
 
     //3rd
     aSuffixes = m_xOrdinal->getOrdinalSuffix(3, aLocale);
-    pStart = aSuffixes.begin();
-    pEnd = aSuffixes.end();
-    pFind = std::find(pStart, pEnd, OUString("rd"));
-    CPPUNIT_ASSERT(pFind != pEnd);
+    pFind = std::find(std::cbegin(aSuffixes), std::cend(aSuffixes), OUString("rd"));
+    CPPUNIT_ASSERT(pFind != std::cend(aSuffixes));
 }
 
 
diff --git a/i18npool/source/transliteration/ignoreIandEfollowedByYa_ja_JP.cxx b/i18npool/source/transliteration/ignoreIandEfollowedByYa_ja_JP.cxx
index 4d09e9f88ef0..5a071a3887fa 100644
--- a/i18npool/source/transliteration/ignoreIandEfollowedByYa_ja_JP.cxx
+++ b/i18npool/source/transliteration/ignoreIandEfollowedByYa_ja_JP.cxx
@@ -77,7 +77,8 @@ ignoreIandEfollowedByYa_ja_JP::foldingImpl( const OUString& inStr, sal_Int32 sta
     if (pOffset) {
         // Allocate nCount length to offset argument.
         pOffset->realloc( nCount );
-        std::iota(pOffset->begin(), pOffset->end(), startPos);
+        auto [begin, end] = toNonConstRange(*pOffset);
+        std::iota(begin, end, startPos);
     }
 
 
diff --git a/i18npool/source/transliteration/ignoreIterationMark_ja_JP.cxx b/i18npool/source/transliteration/ignoreIterationMark_ja_JP.cxx
index 22a600d51f41..bdee0d87d6fa 100644
--- a/i18npool/source/transliteration/ignoreIterationMark_ja_JP.cxx
+++ b/i18npool/source/transliteration/ignoreIterationMark_ja_JP.cxx
@@ -95,7 +95,8 @@ ignoreIterationMark_ja_JP::foldingImpl( const OUString& inStr, sal_Int32 startPo
     if (pOffset) {
         // Allocate nCount length to offset argument.
         pOffset->realloc( nCount );
-        std::iota(pOffset->begin(), pOffset->end(), startPos);
+        auto [begin, end] = toNonConstRange(*pOffset);
+        std::iota(begin, end, startPos);
     }
 
 
diff --git a/i18npool/source/transliteration/ignoreKiKuFollowedBySa_ja_JP.cxx b/i18npool/source/transliteration/ignoreKiKuFollowedBySa_ja_JP.cxx
index f54ff822e4e1..1c64d65104d9 100644
--- a/i18npool/source/transliteration/ignoreKiKuFollowedBySa_ja_JP.cxx
+++ b/i18npool/source/transliteration/ignoreKiKuFollowedBySa_ja_JP.cxx
@@ -38,7 +38,8 @@ ignoreKiKuFollowedBySa_ja_JP::foldingImpl( const OUString& inStr, sal_Int32 star
     if (pOffset) {
         // Allocate nCount length to offset argument.
         pOffset->realloc( nCount );
-        std::iota(pOffset->begin(), pOffset->end(), startPos);
+        auto [begin, end] = toNonConstRange(*pOffset);
+        std::iota(begin, end, startPos);
     }
 
 
diff --git a/i18npool/source/transliteration/ignoreProlongedSoundMark_ja_JP.cxx b/i18npool/source/transliteration/ignoreProlongedSoundMark_ja_JP.cxx
index 4d7f8241a35a..fc87d206dc3e 100644
--- a/i18npool/source/transliteration/ignoreProlongedSoundMark_ja_JP.cxx
+++ b/i18npool/source/transliteration/ignoreProlongedSoundMark_ja_JP.cxx
@@ -300,7 +300,8 @@ ignoreProlongedSoundMark_ja_JP::foldingImpl( const OUString& inStr, sal_Int32 st
     if (pOffset) {
         // Allocate nCount length to offset argument.
         pOffset->realloc( nCount );
-        std::iota(pOffset->begin(), pOffset->end(), startPos);
+        auto [begin, end] = toNonConstRange(*pOffset);
+        std::iota(begin, end, startPos);
     }
 
 
diff --git a/i18npool/source/transliteration/transliterationImpl.cxx b/i18npool/source/transliteration/transliterationImpl.cxx
index 8649eccfc807..6a04ce8cd823 100644
--- a/i18npool/source/transliteration/transliterationImpl.cxx
+++ b/i18npool/source/transliteration/transliterationImpl.cxx
@@ -306,7 +306,7 @@ TransliterationImpl::transliterate( const OUString& inStr, sal_Int32 startPos, s
             tmpStr = bodyCascade[0]->transliterate(tmpStr, 0, nCount, offset);
             if ( startPos )
             {
-                for (sal_Int32 & j : offset)
+                for (sal_Int32 & j : toNonConstRange(offset))
                     j += startPos;
             }
             return tmpStr;
@@ -316,31 +316,22 @@ TransliterationImpl::transliterate( const OUString& inStr, sal_Int32 startPos, s
     {
         OUString tmpStr = inStr.copy(startPos, nCount);
 
-        std::iota(offset.begin(), offset.end(), startPos);
+        auto [begin, end] = toNonConstRange(offset);
+        std::iota(begin, end, startPos);
 
-        sal_Int16 from = 0, to = 1;
-        Sequence<sal_Int32> off[2];
-
-        off[to] = offset;
-        off[from].realloc(nCount);
+        Sequence<sal_Int32> from(nCount);
+        Sequence<sal_Int32> to = offset;
         for (sal_Int32 i = 0; i < numCascade; i++) {
-            tmpStr = bodyCascade[i]->transliterate(tmpStr, 0, nCount, off[from]);
+            tmpStr = bodyCascade[i]->transliterate(tmpStr, 0, nCount, from);
 
             nCount = tmpStr.getLength();
 
-            assert(off[from].getLength() == nCount);
-            std::swap(from, to);
-            // tdf#89665: don't use operator[] to write - too slow!
-            // interestingly gcc 4.9 -Os won't even inline the const operator[]
-            sal_Int32 const*const pFrom(off[from].getConstArray());
-            sal_Int32 *const pTo(off[to].getArray());
-            for (sal_Int32 j = 0; j < nCount; j++)
-            {
-                assert(pTo[j] < off[from].getLength());
-                pTo[j] = pFrom[pTo[j]];
-            }
+            assert(from.getLength() == nCount);
+            from.swap(to);
+            for (sal_Int32& ix : toNonConstRange(to))
+                ix = std::as_const(from)[ix];
         }
-        offset = off[to];
+        offset = to;
         return tmpStr;
     }
 }
@@ -375,7 +366,8 @@ TransliterationImpl::folding( const OUString& inStr, sal_Int32 startPos, sal_Int
     {
         OUString tmpStr = inStr.copy(startPos, nCount);
 
-        std::iota(offset.begin(), offset.end(), startPos);
+        auto [begin, end] = toNonConstRange(offset);
+        std::iota(begin, end, startPos);
 
         sal_Int16 from = 0, to = 1;
         Sequence<sal_Int32> off[2];
diff --git a/i18npool/source/transliteration/transliteration_OneToOne.cxx b/i18npool/source/transliteration/transliteration_OneToOne.cxx
index a030d558d3b8..34f4902f79bb 100644
--- a/i18npool/source/transliteration/transliteration_OneToOne.cxx
+++ b/i18npool/source/transliteration/transliteration_OneToOne.cxx
@@ -68,7 +68,8 @@ transliteration_OneToOne::transliterateImpl( const OUString& inStr, sal_Int32 st
     // Allocate nCount length to offset argument.
     if (pOffset) {
         pOffset->realloc( nCount );
-        std::iota(pOffset->begin(), pOffset->end(), startPos);
+        auto [begin, end] = toNonConstRange(*pOffset);
+        std::iota(begin, end, startPos);
     }
 
     // Translation
diff --git a/i18npool/source/transliteration/transliteration_body.cxx b/i18npool/source/transliteration/transliteration_body.cxx
index 1f4541082435..b0c710c6c696 100644
--- a/i18npool/source/transliteration/transliteration_body.cxx
+++ b/i18npool/source/transliteration/transliteration_body.cxx
@@ -260,8 +260,9 @@ static OUString transliterate_titlecase_Impl(
                xCharClassImpl->toLower( aText, 1, aText.getLength() - 1, rLocale );
         pOffset->realloc( aRes.getLength() );
 
-        sal_Int32* pOffsetInt = std::fill_n(pOffset->begin(), nResolvedLen, 0);
-        std::iota(pOffsetInt, pOffset->end(), 1);
+        auto [begin, end] = toNonConstRange(*pOffset);
+        sal_Int32* pOffsetInt = std::fill_n(begin, nResolvedLen, 0);
+        std::iota(pOffsetInt, end, 1);
     }
     return aRes;
 }
-- 
cgit