strepsirrhini/office-gobmx
9
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

reduce number of SvxUnoShape listeners on SdrModel

Browse source 
when we have a lot of shapes, the number of listeners attached to
SdrModel becomes a serious bottleneck because of the number of
broadcast/notify function calls.

So
(1) make SvxUnoShape listen to the SdrObject
which achieves the same end, and reduces the bottleneck on the SdrModel.
(2) repurpose the maAllIncarnatedObjects set, which tracks all
objects linked to the SdrModel, so we can shut down the associated
UNO shapes during ClearModel. There is no other good way of doing
this, and if we don't do it, we get various use-after-free
issues during shutdown.

This takes the load time of a large DOCX with lots of shapes,
from 35s to 17s on my machine.

Change-Id: I34a6ac35c90de8b7103a7373aafe6e9607cc01c4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169612
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
This commit is contained in:
Noel Grandin 2024-06-26 11:57:42 +02:00
parent c519de2296
commit d900a952eb
5 changed files with 33 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
include/svx/svdmodel.hxx
View file

@ -42,11 +42,7 @@
#include <rtl/ref.hxx>
#include <deque>
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog
#include <unordered_set>
#endif
class OutputDevice;
class SdrOutliner;
@ -153,8 +149,9 @@ struct SdrModelImpl;
class SVXCORE_DLLPUBLIC SdrModel : public SfxBroadcaster, public tools::WeakBase
{
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog:
// We need to keep track of all the SdrObjects linked to this SdrModel so that we can
// shut them down cleanly, otherwise we end up with use-after-free issues.
//
// Use maAllIncarnatedObjects to keep track of all SdrObjects incarnated using this SdrModel
// (what is now possible after the paradigm change that a SdrObject stays at a single SdrModel
// for it's whole lifetime).
@ -174,7 +171,6 @@ class SVXCORE_DLLPUBLIC SdrModel : public SfxBroadcaster, public tools::WeakBase
friend void impAddIncarnatedSdrObjectToSdrModel(SdrObject& rSdrObject, SdrModel& rSdrModel);
friend void impRemoveIncarnatedSdrObjectToSdrModel(SdrObject& rSdrObject, SdrModel& rSdrModel);
std::unordered_set< SdrObject* > maAllIncarnatedObjects;
#endif
protected:
std::vector<rtl::Reference<SdrPage>> maMasterPages;
std::vector<rtl::Reference<SdrPage>> maPages;

2
include/svx/unoshape.hxx
View file

@ -226,7 +226,7 @@ public:
void setMaster( SvxShapeMaster* pMaster );
// SfxListener
virtual void Notify( SfxBroadcaster& rBC, const SfxHint& rHint ) noexcept override;
virtual void Notify( SfxBroadcaster& rBC, const SfxHint& rHint ) noexcept override final;
// XAggregation
virtual css::uno::Any SAL_CALL queryAggregation( const css::uno::Type& aType ) override;

21
svx/source/svdraw/svdmodel.cxx
View file

@ -44,6 +44,7 @@
#include <svx/svdpool.hxx>
#include <svx/svdobj.hxx>
#include <svx/svdotext.hxx>
#include <svx/unoshape.hxx>
#include <textchain.hxx>
#include <svx/svdetc.hxx>
#include <svx/svdoutl.hxx>
@ -222,7 +223,6 @@ SdrModel::~SdrModel()
implDtorClearModel();
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog:
if(!maAllIncarnatedObjects.empty())
{
SAL_WARN("svx",
@ -580,6 +580,25 @@ void SdrModel::ClearModel(bool bCalledFromDestructor)
mbInDestruction = true;
}
// Disconnect all SvxShape's from their SdrObjects to prevent the SdrObjects
// from hanging around and causing use-after-free.
// Make a copy because it might modified during InvalidateSdrObject calls.
std::vector<rtl::Reference<SdrObject>> allObjs(maAllIncarnatedObjects.begin(), maAllIncarnatedObjects.end());
for (const auto & pSdrObj : allObjs)
{
uno::Reference<uno::XInterface> xShape = pSdrObj->getWeakUnoShape().get();
rtl::Reference<SvxShape> pSvxShape = dynamic_cast<SvxShape*>(xShape.get());
// calling getWeakUnoShape so we don't accidentally create new UNO shapes
if (pSvxShape)
pSvxShape->InvalidateSdrObject();
else
{
// because some things like SwXShape don't subclass SvxShape
uno::Reference<lang::XComponent> xComp(xShape, uno::UNO_QUERY);
if (xComp)
xComp->dispose();
}
}
sal_Int32 i;
// delete all drawing pages
sal_Int32 nCount=GetPageCount();

12
svx/source/svdraw/svdobj.cxx
View file

@ -321,8 +321,6 @@ void SdrObject::SetBoundRectDirty()
resetOutRectangle();
}
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog:
void impAddIncarnatedSdrObjectToSdrModel(SdrObject& rSdrObject, SdrModel& rSdrModel)
{
rSdrModel.maAllIncarnatedObjects.insert(&rSdrObject);
@ -334,7 +332,6 @@ void impRemoveIncarnatedSdrObjectToSdrModel(SdrObject& rSdrObject, SdrModel& rSd
assert(false && "SdrObject::~SdrObject: Destructed incarnation of SdrObject not member of this SdrModel (!)");
}
}
#endif
SdrObject::SdrObject(SdrModel& rSdrModel)
: mpFillGeometryDefiningShape(nullptr)
@ -369,10 +366,7 @@ SdrObject::SdrObject(SdrModel& rSdrModel)
m_bIs3DObj=false;
m_bMarkProt=false;
m_bIsUnoObj=false;
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog:
impAddIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
#endif
}
SdrObject::SdrObject(SdrModel& rSdrModel, SdrObject const & rSource)
@ -440,10 +434,7 @@ SdrObject::SdrObject(SdrModel& rSdrModel, SdrObject const & rSource)
m_pGrabBagItem.reset();
if (rSource.m_pGrabBagItem!=nullptr)
m_pGrabBagItem.reset(rSource.m_pGrabBagItem->Clone());
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog:
impAddIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
#endif
}
SdrObject::~SdrObject()
@ -470,10 +461,7 @@ SdrObject::~SdrObject()
m_pGrabBagItem.reset();
mpProperties.reset();
mpViewContact.reset();
#ifdef DBG_UTIL
// SdrObjectLifetimeWatchDog:
impRemoveIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
#endif
}
void SdrObject::acquire() noexcept

31
svx/source/unodraw/unoshape.cxx
View file

@ -192,7 +192,7 @@ SvxShape::~SvxShape() noexcept
if ( mxSdrObject )
{
EndListening(mxSdrObject->getSdrModelFromSdrObject());
mxSdrObject->RemoveListener(*this);
mxSdrObject->setUnoShape(nullptr);
mxSdrObject.clear();
}
@ -205,7 +205,7 @@ void SvxShape::InvalidateSdrObject()
{
if(mxSdrObject)
{
EndListening(mxSdrObject->getSdrModelFromSdrObject());
mxSdrObject->RemoveListener(*this);
mxSdrObject.clear();
}
};
@ -285,7 +285,7 @@ void SvxShape::impl_construct()
{
if ( HasSdrObject() )
{
StartListening(GetSdrObject()->getSdrModelFromSdrObject());
GetSdrObject()->AddListener(*this);
impl_initFromSdrObject();
}
}
@ -355,14 +355,14 @@ void SvxShape::Create( SdrObject* pNewObj, SvxDrawPage* /*pNewPage*/ )
if( HasSdrObject() )
{
EndListening( GetSdrObject()->getSdrModelFromSdrObject() );
GetSdrObject()->RemoveListener( *this );
}
mxSdrObject = pNewObj;
if( HasSdrObject() )
{
StartListening( GetSdrObject()->getSdrModelFromSdrObject() );
GetSdrObject()->AddListener( *this );
}
OSL_ENSURE( !mbIsMultiPropertyCall, "SvxShape::Create: hmm?" );
@ -929,8 +929,7 @@ void SvxShape::Notify( SfxBroadcaster&, const SfxHint& rHint ) noexcept
return;
const SdrHint* pSdrHint = static_cast<const SdrHint*>(&rHint);
// #i55919# SdrHintKind::ObjectChange is only interesting if it's for this object
if ((pSdrHint->GetKind() != SdrHintKind::ModelCleared) &&
(pSdrHint->GetKind() != SdrHintKind::ObjectChange || pSdrHint->GetObject() != mxSdrObject.get() ))
if (pSdrHint->GetKind() != SdrHintKind::ObjectChange || pSdrHint->GetObject() != mxSdrObject.get())
return;
// prevent object being deleted from under us
@ -938,24 +937,12 @@ void SvxShape::Notify( SfxBroadcaster&, const SfxHint& rHint ) noexcept
uno::Reference< uno::XInterface > xSelf( mxSdrObject->getWeakUnoShape() );
if( !xSelf.is() )
{
EndListening(mxSdrObject->getSdrModelFromSdrObject());
mxSdrObject->RemoveListener(*this);
mxSdrObject.clear();
return;
}
if (pSdrHint->GetKind() == SdrHintKind::ObjectChange)
{
updateShapeKind();
}
else // (pSdrHint->GetKind() == SdrHintKind::ModelCleared)
{
EndListening(mxSdrObject->getSdrModelFromSdrObject());
mxSdrObject->setUnoShape(nullptr);
mxSdrObject.clear();
if(!mpImpl->mbDisposing)
dispose();
}
updateShapeKind();
}
// XShape
@ -1190,7 +1177,7 @@ void SAL_CALL SvxShape::dispose()
if (!pObject)
return;
EndListening( pObject->getSdrModelFromSdrObject() );
pObject->RemoveListener( *this );
if ( pObject->IsInserted() && pObject->getSdrPageFromSdrObject() )
{