GetMarkStart/GetMarkEnd is hot

when importing docs with lots of fieldmarks.
But once we have computed the one, computing the other is essentially
free, so instead of doing the computation twice, return both values when
we need both

Change-Id: Icdd9d4ed4cf1e6c9b26fdb20507c4f964f9e9f90
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171853
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins

10 files changed, 60 insertions, 37 deletions

diff --git a/sw/inc/IMark.hxx b/sw/inc/IMark.hxx
index 1a5aed926bfb..3ad7e78213b3 100644
--- a/sw/inc/IMark.hxx
+++ b/sw/inc/IMark.hxx
@@ -72,19 +72,36 @@ namespace sw::mark
         }
         virtual SwPosition& GetMarkStart() const
         {
-            if( !IsExpanded() ) return GetMarkPos( );
-            if ( GetMarkPos( ) < GetOtherMarkPos( ) )
-                return GetMarkPos();
+            SwPosition& rPos1 = GetMarkPos();
+            if( !IsExpanded() )
+                return rPos1;
+            SwPosition& rPos2 = GetOtherMarkPos();
+            if ( rPos1 < rPos2 )
+                return rPos1;
             else
-                return GetOtherMarkPos( );
+                return rPos2;
         }
         virtual SwPosition& GetMarkEnd() const
         {
-            if( !IsExpanded() ) return GetMarkPos();
-            if ( GetMarkPos( ) >= GetOtherMarkPos( ) )
-                return GetMarkPos( );
+            SwPosition& rPos1 = GetMarkPos();
+            if( !IsExpanded() )
+                return rPos1;
+            SwPosition& rPos2 = GetOtherMarkPos();
+            if ( rPos1 >= rPos2 )
+                return rPos1;
             else
-                return GetOtherMarkPos( );
+                return rPos2;
+        }
+        virtual std::pair<SwPosition&,SwPosition&> GetMarkStartEnd() const
+        {
+            SwPosition& rPos1 = GetMarkPos();
+            if( !IsExpanded() )
+                return {rPos1, rPos1};
+            SwPosition& rPos2 = GetOtherMarkPos();
+            if ( rPos1 < rPos2 )
+                return {rPos1, rPos2};
+            else
+                return {rPos2, rPos1};
         }
 
         bool IsCoveringPosition(const SwPosition& rPos) const;
diff --git a/sw/source/core/crsr/bookmark.cxx b/sw/source/core/crsr/bookmark.cxx
index 6f82c913735a..32ec60c3c83c 100644
--- a/sw/source/core/crsr/bookmark.cxx
+++ b/sw/source/core/crsr/bookmark.cxx
@@ -59,8 +59,7 @@ namespace sw::mark
 
     SwPosition FindFieldSep(Fieldmark const& rMark)
     {
-        SwPosition const& rStartPos(rMark.GetMarkStart());
-        SwPosition const& rEndPos(rMark.GetMarkEnd());
+        auto [/*const SwPosition&*/ rStartPos, rEndPos] = rMark.GetMarkStartEnd();
         SwNodes const& rNodes(rStartPos.GetNodes());
         SwNodeOffset const nStartNode(rStartPos.GetNodeIndex());
         SwNodeOffset const nEndNode(rEndPos.GetNodeIndex());
@@ -292,7 +291,8 @@ namespace sw::mark
     // TextFieldmark::InitDoc/lcl_AssureFieldMarksSet.
     bool MarkBase::IsCoveringPosition(const SwPosition& rPos) const
     {
-        return GetMarkStart() <= rPos && rPos < GetMarkEnd();
+        auto [/*const SwPosition&*/ rStartPos, rEndPos] = GetMarkStartEnd();
+        return rStartPos <= rPos && rPos < rEndPos;
     }
 
     void MarkBase::SetMarkPos(const SwPosition& rNewPos)
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index e95b3be234c1..047210fb3631 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -253,8 +253,7 @@ namespace sw
         {
             const ::sw::mark::MarkBase* const pMark = *ppMark;
 
-            const SwPosition& rMarkStart = pMark->GetMarkStart();
-            const SwPosition& rMarkEnd = pMark->GetMarkEnd();
+            auto [/*const SwPosition&*/ rMarkStart, rMarkEnd] = pMark->GetMarkStartEnd();
             // only include marks that are in the range and not touching both start and end
             // - not for annotation or checkbox marks.
             bool const isIncludeStart(
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 1c5a278b97ca..acc0c1c7f4d2 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -1398,10 +1398,11 @@ namespace sw::mark
             m_vFieldmarks.begin(),
             m_vFieldmarks.end(),
             [&rPos] (::sw::mark::MarkBase const*const pMark) {
-                    return pMark->GetMarkStart() == rPos
+                    auto [/*const SwPosition&*/ rStartPos, rEndPos] = pMark->GetMarkStartEnd();
+                    return rStartPos == rPos
                             // end position includes the CH_TXT_ATR_FIELDEND
-                        || (pMark->GetMarkEnd().GetContentIndex() == rPos.GetContentIndex() + 1
-                            && pMark->GetMarkEnd().GetNode() == rPos.GetNode());
+                        || (rEndPos.GetContentIndex() == rPos.GetContentIndex() + 1
+                            && rEndPos.GetNode() == rPos.GetNode());
                 } );
         return (pFieldmark == m_vFieldmarks.end())
             ? nullptr
@@ -1429,22 +1430,23 @@ namespace sw::mark
             return nullptr;
         // we found our first candidate covering the position ...
         auto pMark = *itCurrent;
-        const SwPosition aMarkStart = pMark->GetMarkStart();
-        SwPosition aMarkEnd = pMark->GetMarkEnd();
+        auto aMarkStartEndPair = pMark->GetMarkStartEnd();
+        const SwPosition* pMarkStart = &aMarkStartEndPair.first;
+        const SwPosition* pMarkEnd = &aMarkStartEndPair.second;
         // ... however we still need to check if there is a smaller/'more inner' one with the same start position
         for(++itCurrent; itCurrent != itEnd; ++itCurrent)
         {
-            if((*itCurrent)->GetMarkStart() < aMarkStart)
+            if((*itCurrent)->GetMarkStart() < *pMarkStart)
                 // any following mark (in reverse order) will have an earlier
                 // start and thus can not be more 'inner' than our previous
                 // match, so we are done.
                 break;
             const SwPosition& rCurrentMarkEnd = (*itCurrent)->GetMarkEnd();
-            if(rPos < rCurrentMarkEnd && rCurrentMarkEnd <= aMarkEnd)
+            if(rPos < rCurrentMarkEnd && rCurrentMarkEnd <= *pMarkEnd)
             {
                 // both covering the position and more inner/smaller => use this one instead
                 pMark = *itCurrent;
-                aMarkEnd = rCurrentMarkEnd;
+                pMarkEnd = &rCurrentMarkEnd;
             }
         }
         return pMark;
@@ -1464,12 +1466,15 @@ namespace sw::mark
         // See if any bookmarks after the first hit are closer to rPos.
         ++it;
 
-        for (; it != m_vBookmarks.end() && (*it)->GetMarkStart() <= rPos; ++it)
+        for (; it != m_vBookmarks.end(); ++it)
         {
             // Find the innermost bookmark.
-            if (rPos < (*it)->GetMarkEnd()
-                && (pBookmark->GetMarkStart() < (*it)->GetMarkStart()
-                    || (*it)->GetMarkEnd() < pBookmark->GetMarkEnd()))
+            auto [/*const SwPosition&*/ rMarkStart, rMarkEnd] = (*it)->GetMarkStartEnd();
+            if (rMarkStart > rPos)
+                break;
+            if (rPos < rMarkEnd
+                && (pBookmark->GetMarkStart() < rMarkStart
+                    || rMarkEnd < pBookmark->GetMarkEnd()))
             {
                 pBookmark = *it;
             }
diff --git a/sw/source/core/fields/postithelper.cxx b/sw/source/core/fields/postithelper.cxx
index f9ea8eef4433..7e0712a42365 100644
--- a/sw/source/core/fields/postithelper.cxx
+++ b/sw/source/core/fields/postithelper.cxx
@@ -51,8 +51,7 @@ bool AnnotationMarkCoversCommentAnchor(const sw::mark::MarkBase* pAnnotationMark
         return false;
     }
 
-    const SwPosition& rMarkStart = pAnnotationMark->GetMarkStart();
-    const SwPosition& rMarkEnd = pAnnotationMark->GetMarkEnd();
+    auto [/*const SwPosition&*/ rMarkStart, rMarkEnd] = pAnnotationMark->GetMarkStartEnd();
 
     if (rMarkStart != rAnchorPos)
     {
diff --git a/sw/source/core/inc/crossrefbookmark.hxx b/sw/source/core/inc/crossrefbookmark.hxx
index 3ee770e8d6e0..8242d044f16f 100644
--- a/sw/source/core/inc/crossrefbookmark.hxx
+++ b/sw/source/core/inc/crossrefbookmark.hxx
@@ -44,6 +44,8 @@ namespace sw::mark {
                 { return const_cast<SwPosition&>(*m_oPos1); }
             virtual SwPosition& GetMarkEnd() const override
                 { return const_cast<SwPosition&>(*m_oPos1); }
+            virtual std::pair<SwPosition&,SwPosition&> GetMarkStartEnd() const override
+                { return { const_cast<SwPosition&>(*m_oPos1), const_cast<SwPosition&>(*m_oPos1) }; }
             virtual bool IsExpanded() const override
                 { return false; }
 
diff --git a/sw/source/core/text/porlay.cxx b/sw/source/core/text/porlay.cxx
index c74310dda24a..730be567d39b 100644
--- a/sw/source/core/text/porlay.cxx
+++ b/sw/source/core/text/porlay.cxx
@@ -1033,8 +1033,7 @@ static void InitBookmarks(
                 // start of first[/end of last] extent, and the other one
                 // is outside this merged paragraph, is it deleted or not?
                 // assume "no" because the line break it contains isn't deleted.
-                SwPosition const& rStart(it.first->GetMarkStart());
-                SwPosition const& rEnd(it.first->GetMarkEnd());
+                auto [/*const SwPosition&*/ rStart, rEnd] = it.first->GetMarkStartEnd();
                 assert(&rStart.GetNode() == pNode);
                 while (iter != end)
                 {
@@ -2924,8 +2923,9 @@ void SwScriptInfo::selectHiddenTextProperty(const SwTextNode& rNode,
         {
             // intersect bookmark range with textnode range and add the intersection to rHiddenMulti
 
-            const sal_Int32 nSt =  pBookmark->GetMarkStart().GetContentIndex();
-            const sal_Int32 nEnd = pBookmark->GetMarkEnd().GetContentIndex();
+            auto [/*const SwPosition&*/ rMarkStartPos, rMarkEndPos] = pBookmark->GetMarkStartEnd();
+            const sal_Int32 nSt =  rMarkStartPos.GetContentIndex();
+            const sal_Int32 nEnd = rMarkEndPos.GetContentIndex();
 
             if( nEnd > nSt )
             {
diff --git a/sw/source/core/txtnode/modeltoviewhelper.cxx b/sw/source/core/txtnode/modeltoviewhelper.cxx
index b40899d8ea9d..7b33bb32baa8 100644
--- a/sw/source/core/txtnode/modeltoviewhelper.cxx
+++ b/sw/source/core/txtnode/modeltoviewhelper.cxx
@@ -138,10 +138,11 @@ ModelToViewHelper::ModelToViewHelper(const SwTextNode &rNode,
             assert(pFieldMark->GetMarkStart().GetNode().GetTextNode()->GetText()[pFieldMark->GetMarkStart().GetContentIndex()] != CH_TXT_ATR_FORMELEMENT);
             // getInnerFieldmarkFor may also return one that starts at rNode,0 -
             // skip it, must be handled in loop below
-            if (pFieldMark->GetMarkStart().GetNode() < rNode)
+            auto [/*const SwPosition&*/ rMarkStartPos, rMarkEndPos] = pFieldMark->GetMarkStartEnd();
+            if (rMarkStartPos.GetNode() < rNode)
             {
                 // this can be a nested field's end - skip over those!
-                if (pFieldMark->GetMarkEnd().GetNode() < rNode)
+                if (rMarkEndPos.GetNode() < rNode)
                 {
                     assert(cursor.GetPoint()->GetNode().GetTextNode()->GetText()[cursor.GetPoint()->GetContentIndex()] == CH_TXT_ATR_FIELDEND);
                 }
diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx
index a2dc0ff1408b..8697b3a09235 100644
--- a/sw/source/core/unocore/unoportenum.cxx
+++ b/sw/source/core/unocore/unoportenum.cxx
@@ -144,8 +144,7 @@ namespace
     void lcl_FillBookmark(sw::mark::MarkBase* const pBkmk, const SwNode& rOwnNode, SwDoc& rDoc, SwXBookmarkPortion_ImplList& rBkmArr)
     {
         bool const isExpanded = pBkmk->IsExpanded();
-        const SwPosition& rStartPos = pBkmk->GetMarkStart();
-        const SwPosition& rEndPos = pBkmk->GetMarkEnd();
+        auto [/*const SwPosition&*/ rStartPos, rEndPos] = pBkmk->GetMarkStartEnd();
         // A bookmark where the text was deleted becomes collapsed
         bool const hasOther = isExpanded && rStartPos != rEndPos;
         bool const bStartPosInNode = rStartPos.GetNode() == rOwnNode;
diff --git a/sw/source/core/unocore/unotext.cxx b/sw/source/core/unocore/unotext.cxx
index 2b4b7ad7f35a..b2f4b221e04e 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -2001,9 +2001,10 @@ void SwXText::ConvertCell(
     }
     while (sw::mark::Fieldmark *const pMark = rIDMA.getInnerFieldmarkFor(*aEndCellPam.End()))
     {
-        if (*aStartCellPam.Start() <= pMark->GetMarkStart())
+        auto [rMarkStart, rMarkEnd] = pMark->GetMarkStartEnd();
+        if (*aStartCellPam.Start() <= rMarkStart)
         {
-            if (*aEndCellPam.End() < pMark->GetMarkEnd())
+            if (*aEndCellPam.End() < rMarkEnd)
             {
                 SAL_INFO("sw.uno", "deleting fieldmark overlapping table cell");
                 rIDMA.deleteMark(pMark);