summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNoel Grandin <noel.grandin@collabora.co.uk>2018-06-13 14:19:17 +0200
committerNoel Grandin <noel.grandin@collabora.co.uk>2018-06-14 09:12:25 +0200
commitd70f53a8269be31f0412926afa5ba826faed6633 (patch)
treecb0d391364033ba090e76c837eb5450d4dba48a7
parent76505bbd862b17b9b02a2d6e68bac308890dec70 (diff)
tdf#100894 freeze when editing calc file with bazillions of cond formatting
This does not fix the root cause of this problem, which is simply that the document has a bazillion style sheets. Either the program which exported this document is broken, or our importer is not correctly de- duplicating the imported stylesheets. Anyhow, I made performance improvements until I realised that it was simply going to be impossible to display that many stylesheets in our UI. But still, this bug was useful in flushing out some performance issues. The improvements, in order of decreasing importance are: (*) Use SfxStyleSheetIterator in SvxStyleToolBoxControl::FillStyleBox to avoid an O(n^2) situation where the pool repeatedly marks all the stylesheets as not-used, and then walks the document finding out if a stylesheet is used. Which is a waste of time because we're searching the documents pool, so of course they are all used. (*) Add a virtual method to avoid dynamic_cast (*) return raw pointers instead of returning rtl::Reference by value to avoid unnecessary reference counting. SfxStyleSheetIterator Change-Id: I15ff9c1846d3ed3e6f5655fa44c762f7619d547a Reviewed-on: https://gerrit.libreoffice.org/55751 Tested-by: Jenkins <ci@libreoffice.org> Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
-rw-r--r--include/svl/IndexedStyleSheets.hxx4
-rw-r--r--include/svl/style.hxx6
-rw-r--r--sc/inc/stlsheet.hxx2
-rw-r--r--sc/source/core/data/document.cxx6
-rw-r--r--sc/source/core/data/stlpool.cxx12
-rw-r--r--sd/source/core/stlpool.cxx27
-rw-r--r--svl/source/items/IndexedStyleSheets.cxx10
-rw-r--r--svl/source/items/style.cxx12
-rw-r--r--svx/source/tbxctrls/tbcontrl.cxx12
9 files changed, 46 insertions, 45 deletions
diff --git a/include/svl/IndexedStyleSheets.hxx b/include/svl/IndexedStyleSheets.hxx
index 58b3c26a6d63..eba5cded45bb 100644
--- a/include/svl/IndexedStyleSheets.hxx
+++ b/include/svl/IndexedStyleSheets.hxx
@@ -109,7 +109,7 @@ public:
* @internal
* Method is not const because the returned style sheet is not const
*/
- rtl::Reference< SfxStyleSheetBase >
+ SfxStyleSheetBase*
GetStyleSheetByPosition(unsigned pos);
/** Find the position of a provided style.
@@ -155,7 +155,7 @@ public:
Reindex();
/** Warning: counting for n starts at 0, i.e., the 0th style sheet is the first that is found. */
- rtl::Reference<SfxStyleSheetBase>
+ SfxStyleSheetBase*
GetNthStyleSheetThatMatchesPredicate(unsigned n, StyleSheetPredicate& predicate,
unsigned startAt = 0);
diff --git a/include/svl/style.hxx b/include/svl/style.hxx
index 955cd99a34ef..6f328a2b7143 100644
--- a/include/svl/style.hxx
+++ b/include/svl/style.hxx
@@ -176,6 +176,9 @@ public:
/// If the style has parents, it is _not_ required that the returned item
/// set has parents (i.e. use it for display purposes only).
virtual std::unique_ptr<SfxItemSet> GetItemSetForPreview();
+
+ /// Fix for expensive dynamic_cast
+ virtual bool isScStyleSheet() const { return false; }
};
/* Class to iterate and search on a SfxStyleSheetBasePool */
@@ -243,8 +246,7 @@ protected:
*/
const svl::IndexedStyleSheets&
GetIndexedStyleSheets() const;
- rtl::Reference<SfxStyleSheetBase>
- GetStyleSheetByPositionInIndex(unsigned pos);
+ SfxStyleSheetBase* GetStyleSheetByPositionInIndex(unsigned pos);
public:
SfxStyleSheetBasePool( SfxItemPool& );
diff --git a/sc/inc/stlsheet.hxx b/sc/inc/stlsheet.hxx
index a30efdc7a19d..b1b4ad68e7c7 100644
--- a/sc/inc/stlsheet.hxx
+++ b/sc/inc/stlsheet.hxx
@@ -53,6 +53,8 @@ public:
void SetUsage( ScStyleSheet::Usage eUse ) const { eUsage = eUse; }
ScStyleSheet::Usage GetUsage() const { return eUsage; }
+ /// Fix for expensive dynamic_cast
+ virtual bool isScStyleSheet() const override { return true; }
private:
virtual ~ScStyleSheet() override;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index e550109a9df5..88d0617b038a 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -4956,9 +4956,11 @@ bool ScDocument::IsStyleSheetUsed( const ScStyleSheet& rStyle ) const
for ( const SfxStyleSheetBase* pStyle = aIter.First(); pStyle;
pStyle = aIter.Next() )
{
- const ScStyleSheet* pScStyle = dynamic_cast<const ScStyleSheet*>( pStyle );
- if ( pScStyle )
+ if (pStyle->isScStyleSheet())
+ {
+ const ScStyleSheet* pScStyle = static_cast<const ScStyleSheet*>( pStyle );
pScStyle->SetUsage( ScStyleSheet::NOTUSED );
+ }
}
bool bIsUsed = false;
diff --git a/sc/source/core/data/stlpool.cxx b/sc/source/core/data/stlpool.cxx
index 416c758eb5b7..1ef20908c947 100644
--- a/sc/source/core/data/stlpool.cxx
+++ b/sc/source/core/data/stlpool.cxx
@@ -113,7 +113,7 @@ SfxStyleSheetBase* ScStyleSheetPool::Create( const OUString& rName,
SfxStyleSheetBase* ScStyleSheetPool::Create( const SfxStyleSheetBase& rStyle )
{
- OSL_ENSURE( dynamic_cast<const ScStyleSheet*>( &rStyle) != nullptr, "Invalid StyleSheet-class! :-/" );
+ OSL_ENSURE( rStyle.isScStyleSheet(), "Invalid StyleSheet-class! :-/" );
return new ScStyleSheet( static_cast<const ScStyleSheet&>(rStyle) );
}
@@ -423,14 +423,10 @@ ScStyleSheet* ScStyleSheetPool::FindCaseIns( const OUString& rName, SfxStyleFami
for (/**/;it != aFoundPositions.end(); ++it)
{
- SfxStyleSheetBase *pFound = GetStyleSheetByPositionInIndex(*it).get();
- ScStyleSheet* pSheet = nullptr;
+ SfxStyleSheetBase *pFound = GetStyleSheetByPositionInIndex(*it);
// we do not know what kind of sheets we have.
- pSheet = dynamic_cast<ScStyleSheet*>(pFound);
- if (pSheet != nullptr)
- {
- return pSheet;
- }
+ if (pFound->isScStyleSheet())
+ return static_cast<ScStyleSheet*>(pFound);
}
return nullptr;
}
diff --git a/sd/source/core/stlpool.cxx b/sd/source/core/stlpool.cxx
index 506cdb08a2f4..b73fe06877a2 100644
--- a/sd/source/core/stlpool.cxx
+++ b/sd/source/core/stlpool.cxx
@@ -636,11 +636,10 @@ void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily
for (std::vector<unsigned>::const_iterator it = aSheetsWithFamily.begin();
it != aSheetsWithFamily.end(); ++it )
{
- rtl::Reference<SfxStyleSheetBase> const xSheet =
- rSourcePool.GetStyleSheetByPositionInIndex( *it );
- if( !xSheet.is() )
+ SfxStyleSheetBase* pSheet = rSourcePool.GetStyleSheetByPositionInIndex( *it );
+ if( !pSheet )
continue;
- rtl::OUString aName( xSheet->GetName() );
+ rtl::OUString aName( pSheet->GetName() );
// now check whether we already have a sheet with the same name
std::vector<unsigned> aSheetsWithName = GetIndexedStyleSheets().FindPositionsByName(aName);
@@ -650,9 +649,9 @@ void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily
{
// if we have a rename suffix, try to find a new name
pExistingSheet =
- GetStyleSheetByPositionInIndex(aSheetsWithName.front()).get();
+ GetStyleSheetByPositionInIndex(aSheetsWithName.front());
if (!rRenameSuffix.isEmpty() &&
- pExistingSheet->GetItemSet().Equals(xSheet->GetItemSet(), false))
+ pExistingSheet->GetItemSet().Equals(pSheet->GetItemSet(), false))
{
// we have found a sheet with the same name, but different contents. Try to find a new name.
// If we already have a sheet with the new name, and it is equal to the one in the source pool,
@@ -664,7 +663,7 @@ void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily
aTmpName = aName + rRenameSuffix + OUString::number(nSuffix);
pExistingSheet = Find(aTmpName, eFamily);
nSuffix++;
- } while( pExistingSheet && pExistingSheet->GetItemSet().Equals(xSheet->GetItemSet(), false) );
+ } while( pExistingSheet && pExistingSheet->GetItemSet().Equals(pSheet->GetItemSet(), false) );
aName = aTmpName;
bAddToList = true;
}
@@ -675,28 +674,28 @@ void SdStyleSheetPool::CopySheets(SdStyleSheetPool& rSourcePool, SfxStyleFamily
assert(!Find(aName, eFamily));
rtl::Reference< SfxStyleSheetBase > xNewSheet( &Make( aName, eFamily ) );
- xNewSheet->SetMask( xSheet->GetMask() );
+ xNewSheet->SetMask( pSheet->GetMask() );
// Also set parent relation for copied style sheets
- OUString aParent( xSheet->GetParent() );
+ OUString aParent( pSheet->GetParent() );
if( !aParent.isEmpty() )
aNewStyles.emplace_back( xNewSheet, aParent );
if( !bAddToList )
{
OUString aHelpFile;
- xNewSheet->SetHelpId( aHelpFile, xSheet->GetHelpId( aHelpFile ) );
+ xNewSheet->SetHelpId( aHelpFile, pSheet->GetHelpId( aHelpFile ) );
}
- xNewSheet->GetItemSet().Put( xSheet->GetItemSet() );
+ xNewSheet->GetItemSet().Put( pSheet->GetItemSet() );
rCreatedSheets.emplace_back( static_cast< SdStyleSheet* >( xNewSheet.get() ) );
- aRenamedList.emplace_back( xSheet->GetName(), aName );
+ aRenamedList.emplace_back( pSheet->GetName(), aName );
}
else if (bAddToList)
{
// Add to list - used for renaming
rCreatedSheets.emplace_back( static_cast< SdStyleSheet* >( pExistingSheet ) );
- aRenamedList.emplace_back( xSheet->GetName(), aName );
+ aRenamedList.emplace_back( pSheet->GetName(), aName );
}
}
@@ -939,7 +938,7 @@ void SdStyleSheetPool::UpdateStdNames()
for (std::vector<unsigned>::const_iterator it = aUserDefinedStyles.begin();
it != aUserDefinedStyles.end(); ++it)
{
- SfxStyleSheetBase* pStyle = GetStyleSheetByPositionInIndex(*it).get();
+ SfxStyleSheetBase* pStyle = GetStyleSheetByPositionInIndex(*it);
if( !pStyle->IsUserDefined() )
{
diff --git a/svl/source/items/IndexedStyleSheets.cxx b/svl/source/items/IndexedStyleSheets.cxx
index b74cffb2a2c9..95d579a3231c 100644
--- a/svl/source/items/IndexedStyleSheets.cxx
+++ b/svl/source/items/IndexedStyleSheets.cxx
@@ -164,19 +164,19 @@ IndexedStyleSheets::GetNumberOfStyleSheetsWithPredicate(StyleSheetPredicate& pre
return r;
}
-rtl::Reference<SfxStyleSheetBase>
+SfxStyleSheetBase*
IndexedStyleSheets::GetNthStyleSheetThatMatchesPredicate(
unsigned n,
StyleSheetPredicate& predicate,
unsigned startAt)
{
- rtl::Reference<SfxStyleSheetBase> retval;
+ SfxStyleSheetBase* retval = nullptr;
unsigned matching = 0;
for (VectorType::const_iterator it = mStyleSheets.begin()+startAt; it != mStyleSheets.end(); ++it) {
SfxStyleSheetBase *ssheet = it->get();
if (predicate.Check(*ssheet)) {
if (matching == n) {
- retval = *it;
+ retval = it->get();
break;
}
++matching;
@@ -223,11 +223,11 @@ IndexedStyleSheets::HasStyleSheet(const rtl::Reference< SfxStyleSheetBase >& sty
return false;
}
-rtl::Reference< SfxStyleSheetBase >
+SfxStyleSheetBase*
IndexedStyleSheets::GetStyleSheetByPosition(unsigned pos)
{
if( pos < mStyleSheets.size() )
- return mStyleSheets.at(pos);
+ return mStyleSheets.at(pos).get();
return nullptr;
}
diff --git a/svl/source/items/style.cxx b/svl/source/items/style.cxx
index 9ffd2aa57062..01873c282455 100644
--- a/svl/source/items/style.cxx
+++ b/svl/source/items/style.cxx
@@ -437,7 +437,7 @@ SfxStyleSheetBase* SfxStyleSheetIterator::operator[](sal_uInt16 nIdx)
SfxStyleSheetBase* retval = nullptr;
if( IsTrivialSearch())
{
- retval = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(nIdx).get();
+ retval = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(nIdx);
nCurrentPosition = nIdx;
}
else if(nMask == SfxStyleSearchBits::All)
@@ -491,7 +491,7 @@ SfxStyleSheetBase* SfxStyleSheetIterator::Next()
if (nStyleSheets > newPosition)
{
nCurrentPosition = newPosition;
- retval = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(nCurrentPosition).get();
+ retval = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(nCurrentPosition);
}
}
else if(nMask == SfxStyleSearchBits::All)
@@ -502,8 +502,8 @@ SfxStyleSheetBase* SfxStyleSheetIterator::Next()
if (familyVector.size() > newPosition)
{
nCurrentPosition = newPosition;
- unsigned stylePosition = familyVector.at(newPosition);
- retval = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(stylePosition).get();
+ unsigned stylePosition = familyVector[newPosition];
+ retval = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(stylePosition);
}
}
else
@@ -533,7 +533,7 @@ SfxStyleSheetBase* SfxStyleSheetIterator::Find(const OUString& rStr)
}
unsigned pos = positions.front();
- SfxStyleSheetBase* pStyle = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(pos).get();
+ SfxStyleSheetBase* pStyle = pBasePool->pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(pos);
nCurrentPosition = pos;
pCurrentStyle = pStyle;
return pCurrentStyle;
@@ -971,7 +971,7 @@ SfxStyleSheetBasePool::GetIndexedStyleSheets() const
return *pImpl->mxIndexedStyleSheets;
}
-rtl::Reference<SfxStyleSheetBase>
+SfxStyleSheetBase*
SfxStyleSheetBasePool::GetStyleSheetByPositionInIndex(unsigned pos)
{
return pImpl->mxIndexedStyleSheets->GetStyleSheetByPosition(pos);
diff --git a/svx/source/tbxctrls/tbcontrl.cxx b/svx/source/tbxctrls/tbcontrl.cxx
index 882bb7194566..31ae2b464abd 100644
--- a/svx/source/tbxctrls/tbcontrl.cxx
+++ b/svx/source/tbxctrls/tbcontrl.cxx
@@ -2409,10 +2409,10 @@ void SvxStyleToolBoxControl::FillStyleBox()
SfxStyleSheetBase* pStyle = nullptr;
bool bDoFill = false;
- pStyleSheetPool->SetSearchMask( eFamily, SfxStyleSearchBits::Used );
+ SfxStyleSheetIterator aIter( pStyleSheetPool, eFamily, SfxStyleSearchBits::Used );
// Check whether fill is necessary
- pStyle = pStyleSheetPool->First();
+ pStyle = aIter.First();
//!!! TODO: This condition isn't right any longer, because we always show some default entries
//!!! so the list doesn't show the count
if ( nCount != pBox->GetEntryCount() )
@@ -2425,7 +2425,7 @@ void SvxStyleToolBoxControl::FillStyleBox()
while ( pStyle && !bDoFill )
{
bDoFill = ( pBox->GetEntry(i) != pStyle->GetName() );
- pStyle = pStyleSheetPool->Next();
+ pStyle = aIter.Next();
i++;
}
}
@@ -2436,7 +2436,7 @@ void SvxStyleToolBoxControl::FillStyleBox()
pBox->Clear();
{
- pStyle = pStyleSheetPool->First();
+ pStyle = aIter.First();
if( pImpl->bSpecModeWriter || pImpl->bSpecModeCalc )
{
@@ -2456,7 +2456,7 @@ void SvxStyleToolBoxControl::FillStyleBox()
if( bInsert )
pBox->InsertEntry( aName );
- pStyle = pStyleSheetPool->Next();
+ pStyle = aIter.Next();
}
}
else
@@ -2464,7 +2464,7 @@ void SvxStyleToolBoxControl::FillStyleBox()
while ( pStyle )
{
pBox->InsertEntry( pStyle->GetName() );
- pStyle = pStyleSheetPool->Next();
+ pStyle = aIter.Next();
}
}
}