do not destroy broadcasters and then recreate again (tdf#134268)

Sorting ends tells all listeners on all the sorted cells to end
listening to stop updates, then sorts the cells and starts
listening again. This will cause all broadcasters for the sorted
cells to temporarily stop having any listeners, so they'll be
deleted and removed from the mdds vector (which may additionally
cause moving large parts of the mdds vector repeatedly). And
since all listeners will want to listen again after the sort,
this will all need to be reconstructed. To avoid this,
temporarily block this removal and then later just checks
and remove any possibly left-over broadcasters that ended up
with no listeners.

Change-Id: Ie2d41d9acd7b657cf31a445870ce7f18d28d5ebb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131069
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

14 files changed, 127 insertions, 11 deletions

diff --git a/include/svl/broadcast.hxx b/include/svl/broadcast.hxx
index c1996ccfeda7..eaf4de8ba448 100644
--- a/include/svl/broadcast.hxx
+++ b/include/svl/broadcast.hxx
@@ -59,7 +59,7 @@ public:
     ListenersType& GetAllListeners();
     const ListenersType& GetAllListeners() const;
 
-    bool HasListeners() const;
+    bool HasListeners() const { return (maListeners.size() - mnEmptySlots) > 0; }
 
     /**
      * Listeners and broadcasters are M:N relationship.  If you want to
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index 1ec4aac4204b..1b1ae1e6ab93 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -178,7 +178,8 @@ class ScColumn : protected ScColumnData
     SCCOL           nCol;
     SCTAB           nTab;
 
-    bool mbFiltering; // it is true if there is a filtering in the column
+    bool mbFiltering : 1; // it is true if there is a filtering in the column
+    bool mbEmptyBroadcastersPending : 1; // a broadcaster not removed during EnableDelayDeletingBroadcasters()
 
 friend class ScDocument;                    // for FillInfo
 friend class ScTable;
@@ -633,6 +634,7 @@ public:
 
     void DeleteBroadcasters( sc::ColumnBlockPosition& rBlockPos, SCROW nRow1, SCROW nRow2 );
     void PrepareBroadcastersForDestruction();
+    void DeleteEmptyBroadcasters();
 
     void Broadcast( SCROW nRow );
     void BroadcastCells( const std::vector<SCROW>& rRows, SfxHintId nHint );
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index f78afc46a3da..ddd59d16f3d3 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -525,6 +525,8 @@ private:
     // avoiding repeated calling for the same cells in the given range. The function will be called once
     // later for all the cells in the range.
     std::unordered_map< ScColumn*, std::pair<SCROW, SCROW>> pDelayedStartListeningFormulaCells;
+    // Cells will not delete their broadcasters if delayed, avoiding possible extensive mdds vector changes.
+    bool                bDelayedDeletingBroadcasters;
 
     bool                bLinkFormulaNeedingCheck; // valid only after loading, for ocDde and ocWebservice
 
@@ -1391,6 +1393,10 @@ public:
     /// If true is returned, ScColumn::StartListeningFormulaCells() for the given cells will be performed
     /// later. If false is returned, it needs to be done explicitly.
     bool            CanDelayStartListeningFormulaCells( ScColumn* column, SCROW row1, SCROW row2 );
+    /// If set, cells will not delete their empty broadcasters, avoiding possible extensive mdds
+    /// vector changes. Disabling delay will collect and delete all empty broadcasters.
+    void            EnableDelayDeletingBroadcasters(bool set);
+    bool            IsDelayedDeletingBroadcasters() const { return bDelayedDeletingBroadcasters; }
 
     FormulaError    GetErrCode( const ScAddress& ) const;
 
diff --git a/sc/inc/mtvfunctions.hxx b/sc/inc/mtvfunctions.hxx
index 8f69c05c39fd..9fe394faff57 100644
--- a/sc/inc/mtvfunctions.hxx
+++ b/sc/inc/mtvfunctions.hxx
@@ -23,6 +23,17 @@ struct FuncElseNoOp
     }
 };
 
+template<typename FuncElem, typename Elem>
+struct FuncNotElem
+{
+    FuncElem& func;
+    FuncNotElem(FuncElem& f) : func(f) {}
+    bool operator() (size_t s, Elem elem) const
+    {
+        return !func(s, elem);
+    }
+};
+
 /**
  * Generic algorithm to parse blocks of multi_type_vector either partially
  * or fully.
@@ -512,6 +523,35 @@ FindElement2(
     return PositionType(rStore.end(), 0);
 }
 
+// Efficiently set all elements for which the predicate returns true as empty.
+template<typename Blk1, typename StoreT, typename FuncElem>
+void SetElementsToEmpty1(
+    StoreT& rStore, FuncElem& rFuncElem)
+{
+    typedef std::pair<typename StoreT::const_iterator, typename StoreT::size_type> PositionType;
+
+    for (typename StoreT::iterator it = rStore.begin(); it != rStore.end(); ++it)
+    {
+        if (it->type == Blk1::block_type)
+        {
+            PositionType firstToEmpty = CheckElem<Blk1>(rStore, it, 0, it->size, rFuncElem);
+            if (firstToEmpty.first != rStore.end())
+            {
+                typename StoreT::size_type nFirstOffset = firstToEmpty.second;
+                typename StoreT::size_type nRemainingDataSize = it->size - nFirstOffset;
+                FuncNotElem<FuncElem, typename Blk1::value_type> notFuncElem(rFuncElem);
+                PositionType lastToEmpty = CheckElem<Blk1>(rStore, it, nFirstOffset, nRemainingDataSize,
+                    notFuncElem);
+                typename StoreT::size_type nLastOffset = lastToEmpty.first != rStore.end()
+                    ? lastToEmpty.second - 1 : it->size - 1;
+                it = rStore.set_empty(it, it->position + nFirstOffset, it->position + nLastOffset);
+                // The returned iterator points to the empty elements block.
+                assert(it->type == sc::element_type_empty);
+            }
+        }
+    }
+}
+
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/inc/scopetools.hxx b/sc/inc/scopetools.hxx
index 7489c0874f72..db76c8651633 100644
--- a/sc/inc/scopetools.hxx
+++ b/sc/inc/scopetools.hxx
@@ -93,6 +93,17 @@ public:
     ~DelayStartListeningFormulaCells();
     void set();
 };
+
+/// Wrapper for ScDocument::EnableDelayDeletingBroadcasters()
+class DelayDeletingBroadcasters
+{
+    ScDocument& mDoc;
+    const bool mOldValue;
+
+public:
+    DelayDeletingBroadcasters(ScDocument& doc);
+    ~DelayDeletingBroadcasters();
+};
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 1bc9094232a1..d82dccc0e577 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -1029,6 +1029,7 @@ public:
     SvtBroadcaster* GetBroadcaster( SCCOL nCol, SCROW nRow );
     const SvtBroadcaster* GetBroadcaster( SCCOL nCol, SCROW nRow ) const;
     void DeleteBroadcasters( sc::ColumnBlockPosition& rBlockPos, SCCOL nCol, SCROW nRow1, SCROW nRow2 );
+    void DeleteEmptyBroadcasters();
 
     void FillMatrix( ScMatrix& rMat, SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, svl::SharedStringPool* pPool ) const;
 
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index f14ff4aa8bdc..0598a24858e0 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -87,7 +87,8 @@ ScColumn::ScColumn(ScSheetLimits const & rSheetLimits) :
     mnBlkCountFormula(0),
     nCol( 0 ),
     nTab( 0 ),
-    mbFiltering( false )
+    mbFiltering( false ),
+    mbEmptyBroadcastersPending( false )
 {
     maCells.resize(rSheetLimits.GetMaxRowCount());
 }
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 55bf2a5be132..a7858ab47361 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -1971,6 +1971,28 @@ void ScColumn::PrepareBroadcastersForDestruction()
     }
 }
 
+namespace
+{
+struct BroadcasterNoListenersPredicate
+{
+    bool operator()( size_t, const SvtBroadcaster* broadcaster )
+    {
+        return !broadcaster->HasListeners();
+    }
+};
+
+}
+
+void ScColumn::DeleteEmptyBroadcasters()
+{
+    if(!mbEmptyBroadcastersPending)
+        return;
+    // Clean up after ScDocument::EnableDelayDeletingBroadcasters().
+    BroadcasterNoListenersPredicate predicate;
+    sc::SetElementsToEmpty1<sc::broadcaster_block>( maBroadcasters, predicate );
+    mbEmptyBroadcastersPending = false;
+}
+
 ScPostIt* ScColumn::GetCellNote(SCROW nRow)
 {
     return maCellNotes.get<ScPostIt*>(nRow);
@@ -3268,8 +3290,12 @@ void ScColumn::EndListening( SvtListener& rLst, SCROW nRow )
 
     rLst.EndListening(*pBC);
     if (!pBC->HasListeners())
-        // There is no more listeners for this cell. Remove the broadcaster.
-        maBroadcasters.set_empty(nRow, nRow);
+    {   // There is no more listeners for this cell. Remove the broadcaster.
+        if(GetDoc().IsDelayedDeletingBroadcasters())
+            mbEmptyBroadcastersPending = true;
+        else
+            maBroadcasters.set_empty(nRow, nRow);
+    }
 }
 
 void ScColumn::StartListening( sc::StartListeningContext& rCxt, const ScAddress& rAddress, SvtListener& rLst )
diff --git a/sc/source/core/data/documen2.cxx b/sc/source/core/data/documen2.cxx
index 8cb7b1726376..9c2bbc7c9970 100644
--- a/sc/source/core/data/documen2.cxx
+++ b/sc/source/core/data/documen2.cxx
@@ -163,6 +163,7 @@ ScDocument::ScDocument( ScDocumentMode eMode, SfxObjectShell* pDocShell ) :
         bInDtorClear( false ),
         bExpandRefs( false ),
         bDetectiveDirty( false ),
+        bDelayedDeletingBroadcasters( false ),
         bLinkFormulaNeedingCheck( false ),
         nAsianCompression(CharCompressType::Invalid),
         nAsianKerning(SC_ASIANKERNING_INVALID),
diff --git a/sc/source/core/data/document10.cxx b/sc/source/core/data/document10.cxx
index 2e839d80dba4..1d76747b5d3d 100644
--- a/sc/source/core/data/document10.cxx
+++ b/sc/source/core/data/document10.cxx
@@ -443,6 +443,19 @@ bool ScDocument::CanDelayStartListeningFormulaCells( ScColumn* column, SCROW row
     return true;
 }
 
+void ScDocument::EnableDelayDeletingBroadcasters( bool set )
+{
+    if( bDelayedDeletingBroadcasters == set )
+        return;
+    bDelayedDeletingBroadcasters = set;
+    if( !bDelayedDeletingBroadcasters )
+    {
+        for (auto& rxTab : maTabs)
+            if (rxTab)
+                rxTab->DeleteEmptyBroadcasters();
+    }
+}
+
 bool ScDocument::HasFormulaCell( const ScRange& rRange ) const
 {
     if (!rRange.IsValid())
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 99c673eb5d6a..6d9e966eb39a 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -2522,6 +2522,12 @@ void ScTable::DeleteBroadcasters(
     aCol[nCol].DeleteBroadcasters(rBlockPos, nRow1, nRow2);
 }
 
+void ScTable::DeleteEmptyBroadcasters()
+{
+    for( auto& col : aCol )
+        col->DeleteEmptyBroadcasters();
+}
+
 void ScTable::FillMatrix( ScMatrix& rMat, SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, svl::SharedStringPool* pPool ) const
 {
     size_t nMatCol = 0;
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index 8c8bc4e21500..36d144e7ff0d 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -62,6 +62,7 @@
 #include <reordermap.hxx>
 #include <drwlayer.hxx>
 #include <queryevaluator.hxx>
+#include <scopetools.hxx>
 
 #include <svl/sharedstringpool.hxx>
 
@@ -1777,6 +1778,7 @@ void ScTable::Sort(
     const ScSortParam& rSortParam, bool bKeepQuery, bool bUpdateRefs,
     ScProgress* pProgress, sc::ReorderParam* pUndo )
 {
+    sc::DelayDeletingBroadcasters delayDeletingBroadcasters(GetDoc());
     InitSortCollator( rSortParam );
     bGlobalKeepQuery = bKeepQuery;
 
diff --git a/sc/source/core/tool/scopetools.cxx b/sc/source/core/tool/scopetools.cxx
index 679e613730cb..340d4a4d8a8d 100644
--- a/sc/source/core/tool/scopetools.cxx
+++ b/sc/source/core/tool/scopetools.cxx
@@ -105,6 +105,18 @@ void DelayStartListeningFormulaCells::set()
     mColumn.GetDoc().EnableDelayStartListeningFormulaCells(&mColumn, true);
 }
 
+DelayDeletingBroadcasters::DelayDeletingBroadcasters(ScDocument& doc)
+    : mDoc( doc )
+    , mOldValue( mDoc.IsDelayedDeletingBroadcasters())
+{
+    mDoc.EnableDelayDeletingBroadcasters( true );
+}
+
+DelayDeletingBroadcasters::~DelayDeletingBroadcasters()
+{
+    mDoc.EnableDelayDeletingBroadcasters( mOldValue );
+}
+
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 6c75e0f464c1..54373115c9db 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -173,7 +173,7 @@ void SvtBroadcaster::Remove( SvtListener* p )
         --mnListenersFirstUnsorted;
     }
 
-    if (maListeners.size() - mnEmptySlots == 0)
+    if (!HasListeners())
         ListenersGone();
 }
 
@@ -252,11 +252,6 @@ const SvtBroadcaster::ListenersType& SvtBroadcaster::GetAllListeners() const
     return maListeners;
 }
 
-bool SvtBroadcaster::HasListeners() const
-{
-    return (maListeners.size() - mnEmptySlots) > 0;
-}
-
 void SvtBroadcaster::PrepareForDestruction()
 {
     mbAboutToDie = true;