Re: [Libreoffice] Patch for MarkManager

Hi Björn,

today I managed to improve the patch again. The patch now speeds up
our mailmerge-Szenario enormously and seems to eliminate the
exponential waste of cpu-cycles. Would you please have a look at it?

here some measurings:

MailMerge-Time (in ms) without patch for 10, 50, 100, 150 datasets:
2793, 11821, 28608, 64395
MailMerge-Time (in ms) with first patch for 10, 50, 100, 150 datasets:
2561, 11044, 26226, 55812
MailMerge-Time (in ms) with this patch for 10, 50, 100, 150 datasets:
1838, 7357, 14413, 21558

2011/9/6 Christoph Lutz <chrlutz@googlemail.com>:
> 2011/9/5 Bjoern Michaelsen <bjoern.michaelsen@gmail.com>:
> > On Sun, 4 Sep 2011 15:05:32 +0200
> > Christoph Lutz <chrlutz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> >
> > > this is a patch we wrote during the libreoffice hackfest 2011 for
> > > sw/source/core/inc/MarkManager.hxx and sw/source/core/doc/docbm.cxx
> > > which improves speed of mailmerge. Bjoern already knows details so he
> > > might be the one to review the patch... The patch could be applied
> > > under the lgpl.
> >
> > Patch looks good, Just to clarify before pushing, is it contributed
> > under:
> >  MPL 1.1 / GPLv3+ / LGPLv3+
> > as per http://wiki.documentfoundation.org/License_Policy ?

That's OK!

Best,
Christoph

diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index b88de10..8eb3dd0 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -386,6 +386,7 @@ namespace sw { namespace mark
             pMarkBase->SetName(getUniqueMarkName(pMarkBase->GetName()));
 
         // register mark
+        m_aMarkNamesSet.insert(pMarkBase->GetName());
         lcl_InsertMarkSorted(m_vMarks, pMark);
         switch(eType)
         {
@@ -480,8 +481,10 @@ namespace sw { namespace mark
             " - Mark is not in my doc.");
         if(io_pMark->GetName() == rNewName)
             return true;
-        if(findMark(rNewName) != getMarksEnd())
+        if(hasMark(rNewName))
             return false;
+       m_aMarkNamesSet.erase(dynamic_cast< ::sw::mark::MarkBase* >(io_pMark)->GetName());
+       m_aMarkNamesSet.insert(rNewName);
         dynamic_cast< ::sw::mark::MarkBase* >(io_pMark)->SetName(rNewName);
         return true;
     }
@@ -733,6 +736,7 @@ namespace sw { namespace mark
         //it anymore.
         pMark_t xHoldPastErase = *aI;
         m_vMarks.erase(aI);
+        m_aMarkNamesSet.erase(ppMark->get()->GetName());
     }
 
     void MarkManager::deleteMark(const IMark* const pMark)
@@ -766,6 +770,7 @@ namespace sw { namespace mark
     {
         m_vFieldmarks.clear();
         m_vBookmarks.clear();
+        m_aMarkNamesSet.clear();
 #if OSL_DEBUG_LEVEL > 1
         for(iterator_t pBkmk = m_vMarks.begin();
             pBkmk != m_vMarks.end();
@@ -827,14 +832,25 @@ namespace sw { namespace mark
         OSL_ENSURE(rName.getLength(),
             "<MarkManager::getUniqueMarkName(..)>"
             " - a name should be proposed");
-        if(findMark(rName) == getMarksEnd()) return rName;
+        if(!hasMark(rName)) return rName;
         ::rtl::OUStringBuffer sBuf;
         ::rtl::OUString sTmp;
-        for(sal_Int32 nCnt = 1; nCnt < SAL_MAX_INT32; nCnt++)
+
+        // try the name "<rName>XXX" (where XXX is a number starting from 1) unless there is
+        // a unused name. Due to performance-reasons (especially in mailmerge-Szenarios) there
+        // is a map m_aMarkBasenameMapUniqueOffset which holds the next possible offset (XXX) for
+        // rName (so there is no need to test for nCnt-values smaller than the offset).
+        sal_Int32 nCnt = 1;
+        MarkBasenameMapUniqueOffset::iterator aIter = m_aMarkBasenameMapUniqueOffset.find(rName);
+        if(aIter != m_aMarkBasenameMapUniqueOffset.end()) nCnt = aIter->second;
+        while(nCnt < SAL_MAX_INT32)
         {
             sTmp = sBuf.append(rName).append(nCnt).makeStringAndClear();
-            if(findMark(sTmp) == getMarksEnd()) break;
+            nCnt++;
+            if(!hasMark(sTmp)) break;
         }
+        m_aMarkBasenameMapUniqueOffset[rName] = nCnt;
+
         return sTmp;
     }
 
@@ -845,6 +861,11 @@ namespace sw { namespace mark
         sort(m_vFieldmarks.begin(), m_vFieldmarks.end(), &lcl_MarkOrderingByStart);
     }
 
+    bool MarkManager::hasMark(const ::rtl::OUString& rName) const
+    {
+        return (m_aMarkNamesSet.find(rName) != m_aMarkNamesSet.end());
+    }
+
 }} // namespace ::sw::mark
 
 
diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx
index 415b8e7..be3a1f2 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -31,9 +31,14 @@
 
 #include <IMark.hxx>
 #include <IDocumentMarkAccess.hxx>
+#include <boost/unordered_set.hpp>
+#include <boost/unordered_map.hpp>
 
 namespace sw { namespace mark
 {
+
+    typedef boost::unordered_map<rtl::OUString, sal_Int32, rtl::OUStringHash> 
MarkBasenameMapUniqueOffset;
+
     class MarkManager
         : private ::boost::noncopyable
         , virtual public IDocumentMarkAccess
@@ -72,6 +77,7 @@ namespace sw { namespace mark
             virtual const_iterator_t getMarksEnd() const;
             virtual sal_Int32 getMarksCount() const;
             virtual const_iterator_t findMark(const ::rtl::OUString& rName) const;
+            virtual bool hasMark(const ::rtl::OUString& rName) const;
 
             // bookmarks
             virtual const_iterator_t getBookmarksBegin() const;
@@ -92,6 +98,8 @@ namespace sw { namespace mark
             container_t m_vMarks;
             container_t m_vBookmarks;
             container_t m_vFieldmarks;
+            boost::unordered_set<rtl::OUString, rtl::OUStringHash> m_aMarkNamesSet;
+            MarkBasenameMapUniqueOffset m_aMarkBasenameMapUniqueOffset;
             SwDoc * const m_pDoc;
     };
 }}