Optimised UnoCrsrTbl cleanup for faster doc saving

SwDoc has weak_ptr list to notify UnoCrsr instances
when the doc is about to die. These were updated
when each UnoCrsr instance was destroyed.

The first performance issue is the use of a list.
This no doubt is done to avoid the overhead
of removing items at arbitrary position from a
vector. Performance tests show vector is faster
with a large document and ~10k UnoCrsr instances.

More important, there is no need to clean up
the references as frequently as when each
UnoCrsr is destroyed as they are rarely referenced
at all. Having outdated references is no issue either.

The new logic uses a vector and cleans up only
after saving a document and before saving
UnoCrsr instances.

Saving ODT files is now significantly faster for
large documents (100s of pages).

Change-Id: I3895d9d80d174cda9c94b94837e2149e9fadcb42
Reviewed-on: https://gerrit.libreoffice.org/19604Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Björn Michaelsen <bjoern.michaelsen@canonical.com>

sw/inc/doc.hxx

@@ -1643,7 +1643,18 @@ public:
     void dumpAsXml(struct _xmlTextWriter* = 0) const;
 
     std::set<Color> GetDocColors();
-    std::list< std::weak_ptr<SwUnoCrsr> > mvUnoCrsrTbl;
+    std::vector< std::weak_ptr<SwUnoCrsr> > mvUnoCrsrTbl;
+
+    // Remove expired UnoCrsr weak pointers the document keeps to notify about document death.
+    void cleanupUnoCrsrTbl()
+    {
+        // In most cases we'll remove most of the elements.
+        std::vector< std::weak_ptr<SwUnoCrsr> > unoCrsrTbl;
+        std::copy_if(mvUnoCrsrTbl.begin(), mvUnoCrsrTbl.end(),
+                     std::back_inserter(unoCrsrTbl),
+                     [](const std::weak_ptr<SwUnoCrsr>& pWeakPtr) { return !pWeakPtr.expired(); });
+        std::swap(mvUnoCrsrTbl, unoCrsrTbl);
+    }
 
 private:
     // Copies master header to left / first one, if necessary - used by ChgPageDesc().

sw/source/core/doc/CntntIdxStore.cxx

@@ -378,7 +378,8 @@ void ContentIdxStoreImpl::RestoreFlys(SwDoc* pDoc, updater_t& rUpdater, bool bAu
 
 void ContentIdxStoreImpl::SaveUnoCrsrs(SwDoc* pDoc, sal_uLong nNode, sal_Int32 nContent)
 {
-    for (auto pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
+    pDoc->cleanupUnoCrsrTbl();
+    for (const auto& pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
     {
         auto pUnoCrsr(pWeakUnoCrsr.lock());
         if(!pUnoCrsr)

sw/source/core/doc/doc.cxx

@@ -1678,9 +1678,9 @@ std::shared_ptr<SwUnoCrsr> SwDoc::CreateUnoCrsr( const SwPosition& rPos, bool bT
 {
     std::shared_ptr<SwUnoCrsr> pNew;
     if( bTblCrsr )
-        pNew.reset(new SwUnoTableCrsr( rPos ));
+        pNew = std::make_shared<SwUnoTableCrsr>(rPos);
     else
-        pNew.reset(new SwUnoCrsr( rPos ));
+        pNew = std::make_shared<SwUnoCrsr>(rPos);
 
     mvUnoCrsrTbl.push_back( pNew );
     return pNew;

sw/source/core/doc/doccorr.cxx

@@ -120,8 +120,8 @@ void PaMCorrAbs( const SwPaM& rRange,
                 lcl_PaMCorrAbs( const_cast<SwPaM &>(*pCrsrShell->GetTableCrs()), aStart, aEnd, aNewPos );
         }
     }
-    {
-        for(auto pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
+
+    for(const auto& pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
     {
         auto pUnoCursor(pWeakUnoCrsr.lock());
         if(!pUnoCursor)
@@ -162,7 +162,6 @@ void PaMCorrAbs( const SwPaM& rRange,
             pUnoCursor->ModifyNotification( &aHint, NULL );
         }
     }
-    }
 }
 
 void SwDoc::CorrAbs(const SwNodeIndex& rOldNode,
@@ -274,8 +273,8 @@ void PaMCorrRel( const SwNodeIndex &rOldNode,
                 lcl_PaMCorrRel1( pCrsrShell->GetTableCrs(), pOldNode, aNewPos, nCntIdx );
        }
     }
-    {
-        for(auto pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
+
+    for(const auto& pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
     {
         auto pUnoCrsr(pWeakUnoCrsr.lock());
         if(!pUnoCrsr)
@@ -295,7 +294,6 @@ void PaMCorrRel( const SwNodeIndex &rOldNode,
             }
         }
     }
-    }
 }
 
 void SwDoc::CorrRel(const SwNodeIndex& rOldNode,

sw/source/core/doc/docnew.cxx

@@ -426,8 +426,7 @@ SwDoc::~SwDoc()
     getIDocumentRedlineAccess().GetExtraRedlineTable().DeleteAndDestroyAll();
 
     const sw::DocDisposingHint aHint;
-    std::vector< std::weak_ptr<SwUnoCrsr> > vCursorsToKill(mvUnoCrsrTbl.begin(), mvUnoCrsrTbl.end());
-    for(auto& pWeakCursor : vCursorsToKill)
+    for(const auto& pWeakCursor : mvUnoCrsrTbl)
     {
         auto pCursor(pWeakCursor.lock());
         if(pCursor)

sw/source/core/unocore/unocrsr.cxx

@@ -43,9 +43,6 @@ SwUnoCrsr::~SwUnoCrsr()
     if( !pDoc->IsInDtor() )
     {
         assert(!static_cast<bool>(SwIterator<SwClient, SwUnoCrsr>(*this).First()));
-        // remove the weak_ptr the document keeps to notify about document death
-        pDoc->mvUnoCrsrTbl.remove_if(
-            [this](const std::weak_ptr<SwUnoCrsr>& pWeakPtr) -> bool { return pWeakPtr.lock().get() == this; });
     }
 
     // delete the whole ring

sw/source/uibase/app/docsh.cxx

@@ -516,6 +516,8 @@ bool SwDocShell::SaveAs( SfxMedium& rMedium )
 
         // Increase RSID
         m_pDoc->setRsid( m_pDoc->getRsid() );
+
+        m_pDoc->cleanupUnoCrsrTbl();
     }
     SetError( nErr ? nErr : nVBWarning, OUString(  OSL_LOG_PREFIX  ) );