[Libreoffice-commits] core.git: sw/inc sw/source

Ashod Nakashian ashodnakashian at yahoo.com
Tue Oct 27 06:50:18 PDT 2015


 sw/inc/doc.hxx                       |   13 ++++
 sw/source/core/doc/CntntIdxStore.cxx |    3 -
 sw/source/core/doc/doc.cxx           |    4 -
 sw/source/core/doc/doccorr.cxx       |   94 +++++++++++++++++------------------
 sw/source/core/doc/docnew.cxx        |    3 -
 sw/source/core/unocore/unocrsr.cxx   |    3 -
 sw/source/filter/xml/xmlexp.cxx      |    2 
 sw/source/uibase/app/docsh.cxx       |    2 
 8 files changed, 66 insertions(+), 58 deletions(-)

New commits:
commit f99e8f84f5421076b6c0c10a592e43843eabd8ff
Author: Ashod Nakashian <ashodnakashian at yahoo.com>
Date:   Mon Oct 26 10:03:50 2015 -0400

    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/19604
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Björn Michaelsen <bjoern.michaelsen at canonical.com>

diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx
index 8cdda43..5ee4684 100644
--- a/sw/inc/doc.hxx
+++ b/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().
diff --git a/sw/source/core/doc/CntntIdxStore.cxx b/sw/source/core/doc/CntntIdxStore.cxx
index 0bcfb34..fa55562 100644
--- a/sw/source/core/doc/CntntIdxStore.cxx
+++ b/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)
diff --git a/sw/source/core/doc/doc.cxx b/sw/source/core/doc/doc.cxx
index b78dfe7..87ca0ba 100644
--- a/sw/source/core/doc/doc.cxx
+++ b/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;
diff --git a/sw/source/core/doc/doccorr.cxx b/sw/source/core/doc/doccorr.cxx
index 26b1371..82bd250 100644
--- a/sw/source/core/doc/doccorr.cxx
+++ b/sw/source/core/doc/doccorr.cxx
@@ -120,47 +120,46 @@ void PaMCorrAbs( const SwPaM& rRange,
                 lcl_PaMCorrAbs( const_cast<SwPaM &>(*pCrsrShell->GetTableCrs()), aStart, aEnd, aNewPos );
         }
     }
+
+    for(const auto& pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
     {
-        for(auto pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
-        {
-            auto pUnoCursor(pWeakUnoCrsr.lock());
-            if(!pUnoCursor)
-                continue;
+        auto pUnoCursor(pWeakUnoCrsr.lock());
+        if(!pUnoCursor)
+            continue;
 
-            bool bChange = false; // has the UNO cursor been corrected?
+        bool bChange = false; // has the UNO cursor been corrected?
 
-            // determine whether the UNO cursor will leave it's designated
-            // section
-            bool const bLeaveSection =
-                pUnoCursor->IsRemainInSection() &&
-                ( lcl_FindUnoCrsrSection( aNewPos.nNode.GetNode() ) !=
-                  lcl_FindUnoCrsrSection(
-                      pUnoCursor->GetPoint()->nNode.GetNode() ) );
+        // determine whether the UNO cursor will leave it's designated
+        // section
+        bool const bLeaveSection =
+            pUnoCursor->IsRemainInSection() &&
+            ( lcl_FindUnoCrsrSection( aNewPos.nNode.GetNode() ) !=
+                lcl_FindUnoCrsrSection(
+                    pUnoCursor->GetPoint()->nNode.GetNode() ) );
 
-            for(SwPaM& rPaM : pUnoCursor->GetRingContainer())
-            {
-                bChange |= lcl_PaMCorrAbs( rPaM, aStart, aEnd, aNewPos );
-            }
+        for(SwPaM& rPaM : pUnoCursor->GetRingContainer())
+        {
+            bChange |= lcl_PaMCorrAbs( rPaM, aStart, aEnd, aNewPos );
+        }
 
-            SwUnoTableCrsr *const pUnoTblCrsr =
-                dynamic_cast<SwUnoTableCrsr *>(pUnoCursor.get());
-            if( pUnoTblCrsr )
+        SwUnoTableCrsr *const pUnoTblCrsr =
+            dynamic_cast<SwUnoTableCrsr *>(pUnoCursor.get());
+        if( pUnoTblCrsr )
+        {
+            for(SwPaM& rPaM : (&pUnoTblCrsr->GetSelRing())->GetRingContainer())
             {
-                for(SwPaM& rPaM : (&pUnoTblCrsr->GetSelRing())->GetRingContainer())
-                {
-                    bChange |=
-                        lcl_PaMCorrAbs( rPaM, aStart, aEnd, aNewPos );
-                }
+                bChange |=
+                    lcl_PaMCorrAbs( rPaM, aStart, aEnd, aNewPos );
             }
+        }
 
-            // if a UNO cursor leaves its designated section, we must inform
-            // (and invalidate) said cursor
-            if (bChange && bLeaveSection)
-            {
-                // the UNO cursor has left its section. We need to notify it!
-                SwMsgPoolItem aHint( RES_UNOCURSOR_LEAVES_SECTION );
-                pUnoCursor->ModifyNotification( &aHint, NULL );
-            }
+        // if a UNO cursor leaves its designated section, we must inform
+        // (and invalidate) said cursor
+        if (bChange && bLeaveSection)
+        {
+            // the UNO cursor has left its section. We need to notify it!
+            SwMsgPoolItem aHint( RES_UNOCURSOR_LEAVES_SECTION );
+            pUnoCursor->ModifyNotification( &aHint, NULL );
         }
     }
 }
@@ -274,25 +273,24 @@ void PaMCorrRel( const SwNodeIndex &rOldNode,
                 lcl_PaMCorrRel1( pCrsrShell->GetTableCrs(), pOldNode, aNewPos, nCntIdx );
        }
     }
+
+    for(const auto& pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
     {
-        for(auto pWeakUnoCrsr : pDoc->mvUnoCrsrTbl)
+        auto pUnoCrsr(pWeakUnoCrsr.lock());
+        if(!pUnoCrsr)
+            continue;
+        for(SwPaM& rPaM : pUnoCrsr->GetRingContainer())
         {
-            auto pUnoCrsr(pWeakUnoCrsr.lock());
-            if(!pUnoCrsr)
-                continue;
-            for(SwPaM& rPaM : pUnoCrsr->GetRingContainer())
-            {
-                lcl_PaMCorrRel1( &rPaM, pOldNode, aNewPos, nCntIdx );
-            }
+            lcl_PaMCorrRel1( &rPaM, pOldNode, aNewPos, nCntIdx );
+        }
 
-            SwUnoTableCrsr* pUnoTblCrsr =
-                dynamic_cast<SwUnoTableCrsr*>(pUnoCrsr.get());
-            if( pUnoTblCrsr )
+        SwUnoTableCrsr* pUnoTblCrsr =
+            dynamic_cast<SwUnoTableCrsr*>(pUnoCrsr.get());
+        if( pUnoTblCrsr )
+        {
+            for(SwPaM& rPaM : (&pUnoTblCrsr->GetSelRing())->GetRingContainer())
             {
-                for(SwPaM& rPaM : (&pUnoTblCrsr->GetSelRing())->GetRingContainer())
-                {
-                    lcl_PaMCorrRel1( &rPaM, pOldNode, aNewPos, nCntIdx );
-                }
+                lcl_PaMCorrRel1( &rPaM, pOldNode, aNewPos, nCntIdx );
             }
         }
     }
diff --git a/sw/source/core/doc/docnew.cxx b/sw/source/core/doc/docnew.cxx
index 74ab1f1..93820dc 100644
--- a/sw/source/core/doc/docnew.cxx
+++ b/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)
diff --git a/sw/source/core/unocore/unocrsr.cxx b/sw/source/core/unocore/unocrsr.cxx
index 3aae518..590ac66 100644
--- a/sw/source/core/unocore/unocrsr.cxx
+++ b/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
diff --git a/sw/source/filter/xml/xmlexp.cxx b/sw/source/filter/xml/xmlexp.cxx
index fa980ab..0c7ffd5 100644
--- a/sw/source/filter/xml/xmlexp.cxx
+++ b/sw/source/filter/xml/xmlexp.cxx
@@ -298,7 +298,7 @@ sal_uInt32 SwXMLExport::exportDoc( enum XMLTokenEnum eClass )
                  (RedlineMode_t)(( nRedlineMode & nsRedlineMode_t::REDLINE_SHOW_MASK ) | nsRedlineType_t::REDLINE_INSERT ));
     }
 
-     sal_uInt32 nRet = SvXMLExport::exportDoc( eClass );
+    sal_uInt32 nRet = SvXMLExport::exportDoc( eClass );
 
     // now we can restore the redline mode (if we changed it previously)
     if( bSaveRedline )
diff --git a/sw/source/uibase/app/docsh.cxx b/sw/source/uibase/app/docsh.cxx
index 82c0e61..9cdacee 100644
--- a/sw/source/uibase/app/docsh.cxx
+++ b/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  ) );
 


More information about the Libreoffice-commits mailing list