[Libreoffice-commits] core.git: Branch 'feature/chart-opengl2' - 5 commits - include/svl sc/inc sc/source svl/source
Markus Mohrhard
markus.mohrhard at googlemail.com
Sat Dec 28 08:21:45 PST 2013
include/svl/broadcast.hxx | 19 ++++++
sc/inc/column.hxx | 1
sc/source/core/data/column2.cxx | 15 ++++
sc/source/core/data/table1.cxx | 3
svl/source/notify/broadcast.cxx | 126 +++++++++++++++++++++++-----------------
5 files changed, 113 insertions(+), 51 deletions(-)
New commits:
commit a0c3fe0c8c37fa5df3860afe1f8ef6f369ce4580
Author: Markus Mohrhard <markus.mohrhard at googlemail.com>
Date: Mon Dec 23 23:46:02 2013 +0100
fix crash when erasing entry while iterating through vector
Broadcast might result in calling Remove on the same object which erases
the entry from the listeners vector. If we create a copy we can still
iterate through the vector as all iterators are still valid.
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 425a1e2..19abe38 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -124,7 +124,8 @@ void SvtBroadcaster::Broadcast( const SfxHint &rHint )
Normalize();
ListenersType::iterator dest(maDestructedListeners.begin());
- for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
+ ListenersType aListeners(maListeners); // this copy is important to avoid erasing entries while iterating
+ for (ListenersType::iterator it(aListeners.begin()); it != aListeners.end(); ++it)
{
// skip the destructed ones
while (dest != maDestructedListeners.end() && (*dest < *it))
commit 0c80a98738abe0dfb6e8599c6ac23bb1310d03a3
Author: Eike Rathke <erack at redhat.com>
Date: Mon Dec 23 13:04:15 2013 +0100
iterators are not pointers
Change-Id: Ic9809beead66cf0d0e6a6a28bb97b220fb667578
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 8d3905a..425a1e2 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -79,7 +79,7 @@ SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
Normalize();
ListenersType::iterator dest(maDestructedListeners.begin());
- for (ListenersType::iterator it(maListeners.begin()); it < maListeners.end(); ++it)
+ for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
{
bool bStart = true;
@@ -108,7 +108,7 @@ SvtBroadcaster::~SvtBroadcaster()
// listeners, with the exception of those that already asked to be removed
// during their own destruction
ListenersType::iterator dest(maDestructedListeners.begin());
- for (ListenersType::iterator it(maListeners.begin()); it < maListeners.end(); ++it)
+ for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
{
// skip the destructed ones
while (dest != maDestructedListeners.end() && (*dest < *it))
@@ -124,7 +124,7 @@ void SvtBroadcaster::Broadcast( const SfxHint &rHint )
Normalize();
ListenersType::iterator dest(maDestructedListeners.begin());
- for (ListenersType::iterator it(maListeners.begin()); it < maListeners.end(); ++it)
+ for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
{
// skip the destructed ones
while (dest != maDestructedListeners.end() && (*dest < *it))
commit c2530c5381805b6c3da6a2d58a77af2f9a797568
Author: Jan Holesovsky <kendy at collabora.com>
Date: Sat Dec 21 20:51:29 2013 +0100
Make the broadcasting work even when about to destruct.
Change-Id: Idba9302e1ec5234d3d472cda047c09ba52afd328
diff --git a/include/svl/broadcast.hxx b/include/svl/broadcast.hxx
index 72ac303..a77ebdb 100644
--- a/include/svl/broadcast.hxx
+++ b/include/svl/broadcast.hxx
@@ -82,6 +82,7 @@ private:
bool mbAboutToDie:1;
bool mbDisposing:1;
bool mbNormalized:1;
+ bool mbDestNormalized:1;
};
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 50d68a9..8d3905a 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -21,44 +21,23 @@
#include <svl/listener.hxx>
#include <svl/smplhint.hxx>
-namespace {
-
-class StartListeningHandler : std::unary_function<SvtListener*, void>
+void SvtBroadcaster::Normalize()
{
- SvtBroadcaster& mrBC;
-public:
- StartListeningHandler( SvtBroadcaster& rBC ) : mrBC(rBC) {}
-
- void operator() ( SvtListener* p )
+ if (!mbNormalized)
{
- p->StartListening(mrBC);
+ std::sort(maListeners.begin(), maListeners.end());
+ ListenersType::iterator itUniqueEnd = std::unique(maListeners.begin(), maListeners.end());
+ maListeners.erase(itUniqueEnd, maListeners.end());
+ mbNormalized = true;
}
-};
-
-class NotifyHandler : std::unary_function<SvtListener*, void>
-{
- SvtBroadcaster& mrBC;
- const SfxHint& mrHint;
-public:
- NotifyHandler( SvtBroadcaster& rBC, const SfxHint& rHint ) : mrBC(rBC), mrHint(rHint) {}
- void operator() ( SvtListener* p )
+ if (!mbDestNormalized)
{
- p->Notify(mrBC, mrHint);
+ std::sort(maDestructedListeners.begin(), maDestructedListeners.end());
+ ListenersType::iterator itUniqueEnd = std::unique(maDestructedListeners.begin(), maDestructedListeners.end());
+ maDestructedListeners.erase(itUniqueEnd, maDestructedListeners.end());
+ mbDestNormalized = true;
}
-};
-
-}
-
-void SvtBroadcaster::Normalize()
-{
- if (mbNormalized)
- return;
-
- std::sort(maListeners.begin(), maListeners.end());
- ListenersType::iterator itUniqueEnd = std::unique(maListeners.begin(), maListeners.end());
- maListeners.erase(itUniqueEnd, maListeners.end());
- mbNormalized = true;
}
void SvtBroadcaster::Add( SvtListener* p )
@@ -75,6 +54,7 @@ void SvtBroadcaster::Remove( SvtListener* p )
if (mbAboutToDie)
{
maDestructedListeners.push_back(p);
+ mbDestNormalized = false;
return;
}
@@ -88,12 +68,33 @@ void SvtBroadcaster::Remove( SvtListener* p )
ListenersGone();
}
-SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbDisposing(false), mbNormalized(false) {}
+SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbDisposing(false), mbNormalized(false), mbDestNormalized(false) {}
SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
- maListeners(rBC.maListeners), mbAboutToDie(false), mbDisposing(false), mbNormalized(rBC.mbNormalized)
+ maListeners(rBC.maListeners), maDestructedListeners(rBC.maDestructedListeners),
+ mbAboutToDie(rBC.mbAboutToDie), mbDisposing(false),
+ mbNormalized(rBC.mbNormalized), mbDestNormalized(rBC.mbDestNormalized)
{
- std::for_each(maListeners.begin(), maListeners.end(), StartListeningHandler(*this));
+ if (mbAboutToDie)
+ Normalize();
+
+ ListenersType::iterator dest(maDestructedListeners.begin());
+ for (ListenersType::iterator it(maListeners.begin()); it < maListeners.end(); ++it)
+ {
+ bool bStart = true;
+
+ if (mbAboutToDie)
+ {
+ // skip the destructed ones
+ while (dest != maDestructedListeners.end() && (*dest < *it))
+ ++dest;
+
+ bStart = (dest == maDestructedListeners.end() || *dest != *it);
+ }
+
+ if (bStart)
+ (*it)->StartListening(*this);
+ }
}
SvtBroadcaster::~SvtBroadcaster()
@@ -101,12 +102,6 @@ SvtBroadcaster::~SvtBroadcaster()
mbDisposing = true;
Broadcast( SfxSimpleHint(SFX_HINT_DYING) );
- // normalize the list of listeners than already asked for destruction
- std::sort(maDestructedListeners.begin(), maDestructedListeners.end());
- ListenersType::iterator itUniqueEnd = std::unique(maDestructedListeners.begin(), maDestructedListeners.end());
- maDestructedListeners.erase(itUniqueEnd, maDestructedListeners.end());
-
- // and the list of registered listeners too
Normalize();
// now when both lists are sorted, we can linearly unregister all
@@ -126,12 +121,18 @@ SvtBroadcaster::~SvtBroadcaster()
void SvtBroadcaster::Broadcast( const SfxHint &rHint )
{
- if (mbAboutToDie)
- return;
-
Normalize();
- ListenersType listeners(maListeners);
- std::for_each(listeners.begin(), listeners.end(), NotifyHandler(*this, rHint));
+
+ ListenersType::iterator dest(maDestructedListeners.begin());
+ for (ListenersType::iterator it(maListeners.begin()); it < maListeners.end(); ++it)
+ {
+ // skip the destructed ones
+ while (dest != maDestructedListeners.end() && (*dest < *it))
+ ++dest;
+
+ if (dest == maDestructedListeners.end() || *dest != *it)
+ (*it)->Notify(*this, rHint);
+ }
}
void SvtBroadcaster::ListenersGone() {}
commit dd853ba910ee38ba3c68d2d58967f18ed809ace8
Author: Jan Holesovsky <kendy at collabora.com>
Date: Sat Dec 21 17:26:01 2013 +0100
Don't call EndListening() on already destructed listeners.
Change-Id: I9bda35f2246de9d37077dda33c710b89ee008e5a
diff --git a/include/svl/broadcast.hxx b/include/svl/broadcast.hxx
index d122e02..72ac303 100644
--- a/include/svl/broadcast.hxx
+++ b/include/svl/broadcast.hxx
@@ -70,10 +70,15 @@ public:
* themselves from the broadcaster - the broadcaster will not broadcast
* anything after the PrepareForDesctruction() call anyway.
*/
- void PrepareForDestruction() { mbAboutToDie = true; }
+ void PrepareForDestruction();
private:
ListenersType maListeners;
+
+ /// When the broadcaster is about to die, collect listeners that asked for removal.
+ ListenersType maDestructedListeners;
+
+ /// Indicate that this broadcaster will be destructed (we indicate this on all ScColumn's broadcasters during the ScTable destruction, eg.)
bool mbAboutToDie:1;
bool mbDisposing:1;
bool mbNormalized:1;
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 3575c7f..50d68a9 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -35,18 +35,6 @@ public:
}
};
-class EndListeningHandler : std::unary_function<SvtListener*, void>
-{
- SvtBroadcaster& mrBC;
-public:
- EndListeningHandler( SvtBroadcaster& rBC ) : mrBC(rBC) {}
-
- void operator() ( SvtListener* p )
- {
- p->EndListening(mrBC);
- }
-};
-
class NotifyHandler : std::unary_function<SvtListener*, void>
{
SvtBroadcaster& mrBC;
@@ -81,9 +69,15 @@ void SvtBroadcaster::Add( SvtListener* p )
void SvtBroadcaster::Remove( SvtListener* p )
{
- if (mbAboutToDie || mbDisposing)
+ if (mbDisposing)
return;
+ if (mbAboutToDie)
+ {
+ maDestructedListeners.push_back(p);
+ return;
+ }
+
Normalize();
std::pair<ListenersType::iterator,ListenersType::iterator> r =
std::equal_range(maListeners.begin(), maListeners.end(), p);
@@ -107,8 +101,27 @@ SvtBroadcaster::~SvtBroadcaster()
mbDisposing = true;
Broadcast( SfxSimpleHint(SFX_HINT_DYING) );
- // unregister all listeners.
- std::for_each(maListeners.begin(), maListeners.end(), EndListeningHandler(*this));
+ // normalize the list of listeners than already asked for destruction
+ std::sort(maDestructedListeners.begin(), maDestructedListeners.end());
+ ListenersType::iterator itUniqueEnd = std::unique(maDestructedListeners.begin(), maDestructedListeners.end());
+ maDestructedListeners.erase(itUniqueEnd, maDestructedListeners.end());
+
+ // and the list of registered listeners too
+ Normalize();
+
+ // now when both lists are sorted, we can linearly unregister all
+ // listeners, with the exception of those that already asked to be removed
+ // during their own destruction
+ ListenersType::iterator dest(maDestructedListeners.begin());
+ for (ListenersType::iterator it(maListeners.begin()); it < maListeners.end(); ++it)
+ {
+ // skip the destructed ones
+ while (dest != maDestructedListeners.end() && (*dest < *it))
+ ++dest;
+
+ if (dest == maDestructedListeners.end() || *dest != *it)
+ (*it)->EndListening(*this);
+ }
}
void SvtBroadcaster::Broadcast( const SfxHint &rHint )
@@ -133,4 +146,10 @@ bool SvtBroadcaster::HasListeners() const
return !maListeners.empty();
}
+void SvtBroadcaster::PrepareForDestruction()
+{
+ mbAboutToDie = true;
+ maDestructedListeners.reserve(maListeners.size());
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
commit 5d96a8af9f1c90d7a9a5ff29a8294bc185e2aa9c
Author: Jan Holesovsky <kendy at collabora.com>
Date: Sat Dec 21 01:41:18 2013 +0100
Speedup destruction of sheets with too many listeners & broadcasters.
Listeners and broadcasters are M:N relationship. If you want to
destruct them, you easily end up in O(M*N) situation; where for every
listener, you iterate all broadcasters, to remove that one listener.
To avoid that, announce to the broadcasters that they are going to die, and the
listeners do not have to bother with removing themselves from the broadcaster.
The broadcaster will not broadcast anything after the PrepareForDesctruction()
call anyway.
Change-Id: I68d78b23e73bcbb944de9139448b2c20dfa14f62
diff --git a/include/svl/broadcast.hxx b/include/svl/broadcast.hxx
index 6a6bc03..d122e02 100644
--- a/include/svl/broadcast.hxx
+++ b/include/svl/broadcast.hxx
@@ -60,8 +60,21 @@ public:
bool HasListeners() const;
+ /**
+ * Listeners and broadcasters are M:N relationship. If you want to
+ * destruct them, you easily end up in O(M*N) situation; where for every
+ * listener, you iterate all broadcasters, to remove that one listener.
+ *
+ * To avoid that, use this call to announce to the broadcaster it is going
+ * to die, and the listeners do not have to bother with removing
+ * themselves from the broadcaster - the broadcaster will not broadcast
+ * anything after the PrepareForDesctruction() call anyway.
+ */
+ void PrepareForDestruction() { mbAboutToDie = true; }
+
private:
ListenersType maListeners;
+ bool mbAboutToDie:1;
bool mbDisposing:1;
bool mbNormalized:1;
};
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index 55b3847..b7f48db 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -491,6 +491,7 @@ public:
SvtBroadcaster* GetBroadcaster( SCROW nRow );
const SvtBroadcaster* GetBroadcaster( SCROW nRow ) const;
void DeleteBroadcasters( sc::ColumnBlockPosition& rBlockPos, SCROW nRow1, SCROW nRow2 );
+ void PrepareBroadcastersForDestruction();
bool HasBroadcaster() const;
void Broadcast( SCROW nRow );
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 307d42b..2092406b 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -1962,6 +1962,21 @@ void ScColumn::DeleteBroadcasters( sc::ColumnBlockPosition& rBlockPos, SCROW nRo
maBroadcasters.set_empty(rBlockPos.miBroadcasterPos, nRow1, nRow2);
}
+void ScColumn::PrepareBroadcastersForDestruction()
+{
+ sc::BroadcasterStoreType::iterator itPos = maBroadcasters.begin(), itPosEnd = maBroadcasters.end();
+ for (; itPos != itPosEnd; ++itPos)
+ {
+ if (itPos->type == sc::element_type_broadcaster)
+ {
+ sc::broadcaster_block::iterator it = sc::broadcaster_block::begin(*itPos->data);
+ sc::broadcaster_block::iterator itEnd = sc::broadcaster_block::end(*itPos->data);
+ for (; it != itEnd; ++it)
+ (*it)->PrepareForDestruction();
+ }
+ }
+}
+
bool ScColumn::HasBroadcaster() const
{
sc::BroadcasterStoreType::const_iterator it = maBroadcasters.begin(), itEnd = maBroadcasters.end();
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 4eed3b0..37417ac 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -341,6 +341,9 @@ ScTable::~ScTable()
delete mpRangeName;
delete pDBDataNoName;
DestroySortCollator();
+
+ for (SCCOL k=0; k<=MAXCOL; k++)
+ aCol[k].PrepareBroadcastersForDestruction();
}
void ScTable::GetName( OUString& rName ) const
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 24b5790..3575c7f 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -81,7 +81,7 @@ void SvtBroadcaster::Add( SvtListener* p )
void SvtBroadcaster::Remove( SvtListener* p )
{
- if (mbDisposing)
+ if (mbAboutToDie || mbDisposing)
return;
Normalize();
@@ -94,10 +94,10 @@ void SvtBroadcaster::Remove( SvtListener* p )
ListenersGone();
}
-SvtBroadcaster::SvtBroadcaster() : mbDisposing(false), mbNormalized(false) {}
+SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbDisposing(false), mbNormalized(false) {}
SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
- maListeners(rBC.maListeners), mbDisposing(false), mbNormalized(rBC.mbNormalized)
+ maListeners(rBC.maListeners), mbAboutToDie(false), mbDisposing(false), mbNormalized(rBC.mbNormalized)
{
std::for_each(maListeners.begin(), maListeners.end(), StartListeningHandler(*this));
}
@@ -113,6 +113,9 @@ SvtBroadcaster::~SvtBroadcaster()
void SvtBroadcaster::Broadcast( const SfxHint &rHint )
{
+ if (mbAboutToDie)
+ return;
+
Normalize();
ListenersType listeners(maListeners);
std::for_each(listeners.begin(), listeners.end(), NotifyHandler(*this, rHint));
More information about the Libreoffice-commits
mailing list