diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2018-10-01 14:31:00 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2018-10-02 12:29:51 +0200 |
commit | 4a290888580474f9542f185091bb2a6fcf4e9a53 (patch) | |
tree | e61c741d2ee1b200a8766fe7d179caedbf810ee8 | |
parent | 1019fc891964b97c1d43e7113a7d64e5d8d56084 (diff) |
SvtBroadcaster unify the normal and PrepareForDestruction paths
since this approach is better in that it avoids O(n^2) behaviour
of lots of listeners deleting from a std::vector, lets just
always use this approach.
Change-Id: I9204996ee8c9379ac71dfc168a6c1fc653e63a8e
Reviewed-on: https://gerrit.libreoffice.org/61204
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
-rw-r--r-- | svl/source/notify/broadcast.cxx | 72 |
1 files changed, 40 insertions, 32 deletions
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx index 626a48a332e9..7e67de0bcdf1 100644 --- a/svl/source/notify/broadcast.cxx +++ b/svl/source/notify/broadcast.cxx @@ -40,9 +40,8 @@ void SvtBroadcaster::Normalize() const void SvtBroadcaster::Add( SvtListener* p ) { - assert(!mbDisposing && "called inside my own destructor?"); - assert(!mbAboutToDie && "called after PrepareForDestruction()?"); - if (mbDisposing || mbAboutToDie) + assert(!mbAboutToDie && "called inside my own destructor / after PrepareForDestruction()?"); + if (mbAboutToDie) return; // only reset mbNormalized if we are going to become unsorted if (!maListeners.empty() && maListeners.back() > p) @@ -52,13 +51,12 @@ void SvtBroadcaster::Add( SvtListener* p ) void SvtBroadcaster::Remove( SvtListener* p ) { - if (mbDisposing) - return; - if (mbAboutToDie) { + // only reset mbDestNormalized if we are going to become unsorted + if (!maDestructedListeners.empty() && maDestructedListeners.back() > p) + mbDestNormalized = false; maDestructedListeners.push_back(p); - mbDestNormalized = false; return; } @@ -75,14 +73,13 @@ void SvtBroadcaster::Remove( SvtListener* p ) ListenersGone(); } -SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbDisposing(false), mbNormalized(true), mbDestNormalized(true) {} +SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbNormalized(true), mbDestNormalized(true) {} SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) : - mbAboutToDie(false), mbDisposing(false), + mbAboutToDie(false), mbNormalized(true), mbDestNormalized(true) { - assert(!rBC.mbAboutToDie && "copying an object marked with PrepareForDestruction()?"); - assert(!rBC.mbDisposing && "copying an object that is in it's destructor?"); + assert(!rBC.mbAboutToDie && "copying an object marked with PrepareForDestruction() / that is in it's destructor?"); rBC.Normalize(); // so that insert into ourself is in-order, and therefore we do not need to Normalize() maListeners.reserve(rBC.maListeners.size()); @@ -92,41 +89,52 @@ SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) : SvtBroadcaster::~SvtBroadcaster() { - mbDisposing = true; - Broadcast( SfxHint(SfxHintId::Dying) ); + PrepareForDestruction(); + + Normalize(); + + { + SfxHint aHint(SfxHintId::Dying); + ListenersType::const_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(aHint); + } + } 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::const_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)->BroadcasterDying(*this); + ListenersType::const_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)->BroadcasterDying(*this); + } } } void SvtBroadcaster::Broadcast( const SfxHint &rHint ) { - Normalize(); + assert(!mbAboutToDie && "broadcasting after PrepareForDestruction() / from inside my own destructor?"); + if (mbAboutToDie) + return; - ListenersType::const_iterator dest(maDestructedListeners.begin()); 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)) - ++dest; - - if (dest == maDestructedListeners.end() || *dest != *it) - (*it)->Notify(rHint); - } + for (SvtListener * p : aListeners) + p->Notify(rHint); } void SvtBroadcaster::ListenersGone() {} |