svl/qa/unit/items/test_itempool.cxx | 18 +++----
svl/source/inc/poolio.hxx | 13 ++---
svl/source/items/itempool.cxx | 91 ++++++++++++++----------------------
svl/source/items/poolio.cxx | 5 -
4 files changed, 55 insertions(+), 72 deletions(-)
New commits:
commit bcb0c9b4bee1d943d9c60f9d4512dba901f85f54
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Wed Apr 17 15:39:06 2019 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Sat Apr 20 08:17:12 2019 +0200
flatten SfxItemPool_Impl (tdf#81765 related)
Flatten the vector of SfxPoolItemArray_Impl, to reduce pointer chasing.
This struct is movable, etc, so no need to allocate it separately on the
heap.
Change-Id: I794b4356660e9cd0e63bc98b011f58162a838662
Reviewed-on: https://gerrit.libreoffice.org/70884
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/svl/qa/unit/items/test_itempool.cxx
b/svl/qa/unit/items/test_itempool.cxx
index 9b5528532515..e08c77d01378 100644
--- a/svl/qa/unit/items/test_itempool.cxx
+++ b/svl/qa/unit/items/test_itempool.cxx
@@ -43,17 +43,17 @@ void PoolItemTest::testPool()
SfxItemPool *pPool = new SfxItemPool("testpool", 1, 4, aItems);
SfxItemPool_Impl *pImpl = SfxItemPool_Impl::GetImpl(pPool);
CPPUNIT_ASSERT(pImpl != nullptr);
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), pImpl->maPoolItems.size());
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4),
pImpl->maPoolItemArrays.size());
// Poolable
SfxVoidItem aItemOne( 1 );
SfxVoidItem aNotherOne( 1 );
{
- CPPUNIT_ASSERT(!pImpl->maPoolItems[0]);
+ CPPUNIT_ASSERT(pImpl->maPoolItemArrays[0].empty());
const SfxPoolItem &rVal = pPool->Put(aItemOne);
CPPUNIT_ASSERT(bool(rVal == aItemOne));
- CPPUNIT_ASSERT(pImpl->maPoolItems[0] != nullptr);
+ CPPUNIT_ASSERT(!pImpl->maPoolItemArrays[0].empty());
const SfxPoolItem &rVal2 = pPool->Put(aNotherOne);
CPPUNIT_ASSERT(bool(rVal2 == rVal));
CPPUNIT_ASSERT_EQUAL(&rVal, &rVal2);
@@ -69,10 +69,10 @@ void PoolItemTest::testPool()
SfxVoidItem aItemTwo( 2 );
SfxVoidItem aNotherTwo( 2 );
{
- CPPUNIT_ASSERT(!pImpl->maPoolItems[1]);
+ CPPUNIT_ASSERT(pImpl->maPoolItemArrays[1].empty());
const SfxPoolItem &rVal = pPool->Put(aItemTwo);
CPPUNIT_ASSERT(bool(rVal == aItemTwo));
- CPPUNIT_ASSERT(pImpl->maPoolItems[1] != nullptr);
+ CPPUNIT_ASSERT(!pImpl->maPoolItemArrays[1].empty());
const SfxPoolItem &rVal2 = pPool->Put(aNotherTwo);
CPPUNIT_ASSERT(bool(rVal2 == rVal));
@@ -84,12 +84,12 @@ void PoolItemTest::testPool()
SfxVoidItem aNotherFour(4);
const SfxPoolItem &rKeyFour = pPool->Put(aRemoveFour);
pPool->Put(aNotherFour);
- CPPUNIT_ASSERT(pImpl->maPoolItems[3]->size() > 0);
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2),
pImpl->maPoolItems[3]->size());
+ CPPUNIT_ASSERT(pImpl->maPoolItemArrays[3].size() > 0);
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2),
pImpl->maPoolItemArrays[3].size());
pPool->Remove(rKeyFour);
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1),
pImpl->maPoolItems[3]->size());
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1),
pImpl->maPoolItemArrays[3].size());
pPool->Put(aNotherFour);
- CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2),
pImpl->maPoolItems[3]->size());
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2),
pImpl->maPoolItemArrays[3].size());
}
diff --git a/svl/source/inc/poolio.hxx b/svl/source/inc/poolio.hxx
index eb78be10e71a..8eef10c5af96 100644
--- a/svl/source/inc/poolio.hxx
+++ b/svl/source/inc/poolio.hxx
@@ -43,21 +43,22 @@ struct SfxPoolItemArray_Impl
private:
o3tl::sorted_vector<SfxPoolItem*> maPoolItemSet;
public:
- o3tl::sorted_vector<SfxPoolItem*>::const_iterator begin() { return
maPoolItemSet.begin(); }
- o3tl::sorted_vector<SfxPoolItem*>::const_iterator end() { return
maPoolItemSet.end(); }
+ o3tl::sorted_vector<SfxPoolItem*>::const_iterator begin() const { return
maPoolItemSet.begin(); }
+ o3tl::sorted_vector<SfxPoolItem*>::const_iterator end() const { return
maPoolItemSet.end(); }
/// clear array of PoolItem variants after all PoolItems are deleted
/// or all ref counts are decreased
void clear();
size_t size() const {return maPoolItemSet.size();}
+ bool empty() const {return maPoolItemSet.empty();}
void insert(SfxPoolItem* pItem) { maPoolItemSet.insert(pItem); }
- o3tl::sorted_vector<SfxPoolItem*>::const_iterator find(SfxPoolItem* pItem)
{ return maPoolItemSet.find(pItem); }
+ o3tl::sorted_vector<SfxPoolItem*>::const_iterator find(SfxPoolItem* pItem)
const { return maPoolItemSet.find(pItem); }
void erase(o3tl::sorted_vector<SfxPoolItem*>::const_iterator it) { return
maPoolItemSet.erase(it); }
};
struct SfxItemPool_Impl
{
SfxBroadcaster aBC;
- std::vector<std::unique_ptr<SfxPoolItemArray_Impl>> maPoolItems;
+ std::vector<SfxPoolItemArray_Impl> maPoolItemArrays;
std::vector<SfxItemPoolUser*> maSfxItemPoolUsers; /// ObjectUser section
OUString aName;
std::vector<SfxPoolItem*> maPoolDefaults;
@@ -70,7 +71,7 @@ struct SfxItemPool_Impl
MapUnit eDefMetric;
SfxItemPool_Impl( SfxItemPool* pMaster, const OUString& rName, sal_uInt16
nStart, sal_uInt16 nEnd )
- : maPoolItems(nEnd - nStart + 1)
+ : maPoolItemArrays(nEnd - nStart + 1)
, aName(rName)
, maPoolDefaults(nEnd - nStart + 1)
, mpStaticDefaults(nullptr)
@@ -90,7 +91,7 @@ struct SfxItemPool_Impl
void DeleteItems()
{
- maPoolItems.clear();
+ maPoolItemArrays.clear();
maPoolDefaults.clear();
mpPoolRanges.reset();
}
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index 3a0babf6a857..b407889db8e0 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -250,7 +250,7 @@ void SfxItemPool::SetDefaults( std::vector<SfxPoolItem*>*
pDefaults )
assert( ((*pImpl->mpStaticDefaults)[n]->Which() == n +
pImpl->mnStart)
&& "static defaults not sorted" );
(*pImpl->mpStaticDefaults)[n]->SetKind(SfxItemKind::StaticDefault);
- DBG_ASSERT( !(pImpl->maPoolItems[n]), "defaults with setitems with
items?!" );
+ DBG_ASSERT( pImpl->maPoolItemArrays[n].empty(), "defaults with
setitems with items?!" );
}
}
}
@@ -331,7 +331,7 @@ void SfxItemPool::ReleaseDefaults
SfxItemPool::~SfxItemPool()
{
- if ( !pImpl->maPoolItems.empty() && !pImpl->maPoolDefaults.empty() )
+ if ( !pImpl->maPoolItemArrays.empty() && !pImpl->maPoolDefaults.empty() )
Delete();
if (pImpl->mpMaster != nullptr && pImpl->mpMaster != this)
@@ -374,8 +374,8 @@ void SfxItemPool::SetSecondaryPool( SfxItemPool *pPool )
if ( pImpl->mpSecondary )
{
#ifdef DBG_UTIL
- if (pImpl->mpStaticDefaults != nullptr && !pImpl->maPoolItems.empty()
- && !pImpl->mpSecondary->pImpl->maPoolItems.empty())
+ if (pImpl->mpStaticDefaults != nullptr &&
!pImpl->maPoolItemArrays.empty()
+ && !pImpl->mpSecondary->pImpl->maPoolItemArrays.empty())
// Delete() did not yet run?
{
// Does the Master have SetItems?
@@ -385,11 +385,11 @@ void SfxItemPool::SetSecondaryPool( SfxItemPool *pPool )
// Detached Pools must be empty
bool bOK = bHasSetItems;
- for (auto const& rSecArrayPtr :
pImpl->mpSecondary->pImpl->maPoolItems)
+ for (auto const& rSecArray :
pImpl->mpSecondary->pImpl->maPoolItemArrays)
{
if (!bOK)
break;
- if (rSecArrayPtr && rSecArrayPtr->size()>0)
+ if (rSecArray.size()>0)
{
SAL_WARN("svl.items", "old secondary pool: " <<
pImpl->mpSecondary->pImpl->aName
<< " of pool: " << pImpl->aName << " must
be empty.");
@@ -463,7 +463,7 @@ SfxItemPool* SfxItemPool::Clone() const
void SfxItemPool::Delete()
{
// Already deleted?
- if (pImpl->maPoolItems.empty() || pImpl->maPoolDefaults.empty())
+ if (pImpl->maPoolItemArrays.empty() || pImpl->maPoolDefaults.empty())
return;
// Inform e.g. running Requests
@@ -480,17 +480,14 @@ void SfxItemPool::Delete()
if (dynamic_cast<const SfxSetItem*>(pStaticDefaultItem))
{
// SfxSetItem found, remove PoolItems (and defaults) with same
ID
- auto& rArrayPtr = pImpl->maPoolItems[n];
- if (rArrayPtr)
+ auto& rArray = pImpl->maPoolItemArrays[n];
+ for (auto& rItemPtr : rArray)
{
- for (auto& rItemPtr : *rArrayPtr)
- {
- ReleaseRef(*rItemPtr, rItemPtr->GetRefCount()); // for
RefCount check in dtor
- delete rItemPtr;
- }
- rArrayPtr->clear();
- // let pImpl->DeleteItems() delete item arrays in
maPoolItems
+ ReleaseRef(*rItemPtr, rItemPtr->GetRefCount()); // for
RefCount check in dtor
+ delete rItemPtr;
}
+ rArray.clear();
+ // let pImpl->DeleteItems() delete item arrays in maPoolItems
auto& rItemPtr = pImpl->maPoolDefaults[n];
if (rItemPtr)
{
@@ -505,19 +502,17 @@ void SfxItemPool::Delete()
}
// now remove remaining PoolItems (and defaults) who didn't have SetItems
- for (auto& rArrayPtr : pImpl->maPoolItems)
+ for (auto& rArray : pImpl->maPoolItemArrays)
{
- if (rArrayPtr)
+ for (auto& rItemPtr : rArray)
{
- for (auto& rItemPtr : *rArrayPtr)
- {
- ReleaseRef(*rItemPtr, rItemPtr->GetRefCount()); // for
RefCount check in dtor
- delete rItemPtr;
- }
- rArrayPtr->clear();
- // let pImpl->DeleteItems() delete item arrays in maPoolItems
+ ReleaseRef(*rItemPtr, rItemPtr->GetRefCount()); // for RefCount
check in dtor
+ delete rItemPtr;
}
+ rArray.clear();
+ // let pImpl->DeleteItems() delete item arrays in maPoolItems
}
+ pImpl->maPoolItemArrays.clear();
// default items
for (auto rItemPtr : pImpl->maPoolDefaults)
{
@@ -613,12 +608,7 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem&
rItem, sal_uInt16 nWhich
typeid(rItem) == typeid(GetDefaultItem(nWhich)));
const sal_uInt16 nIndex = GetIndex_Impl(nWhich);
- SfxPoolItemArray_Impl* pItemArr = pImpl->maPoolItems[nIndex].get();
- if (!pItemArr)
- {
- pImpl->maPoolItems[nIndex].reset(new SfxPoolItemArray_Impl);
- pItemArr = pImpl->maPoolItems[nIndex].get();
- }
+ SfxPoolItemArray_Impl& rItemArr = pImpl->maPoolItemArrays[nIndex];
// Is this a 'poolable' item - ie. should we re-use and return
// the same underlying item for equivalent (==) SfxPoolItems?
@@ -627,10 +617,10 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem&
rItem, sal_uInt16 nWhich
// if is already in a pool, then it is worth checking if it is in this
one.
if ( IsPooledItem(&rItem) )
{
- auto it = pItemArr->find(const_cast<SfxPoolItem *>(&rItem));
+ auto it = rItemArr.find(const_cast<SfxPoolItem *>(&rItem));
// 1. search for an identical pointer in the pool
- if (it != pItemArr->end())
+ if (it != rItemArr.end())
{
AddRef(rItem);
return rItem;
@@ -638,7 +628,7 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem&
rItem, sal_uInt16 nWhich
}
// 2. search for an item with matching attributes.
- for (auto itr = pItemArr->begin(); itr != pItemArr->end(); ++itr)
+ for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr)
{
if (**itr == rItem)
{
@@ -662,8 +652,8 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem&
rItem, sal_uInt16 nWhich
AddRef( *pNewItem );
// 4. finally insert into the pointer array
- assert( pItemArr->find(pNewItem) == pItemArr->end() );
- pItemArr->insert( pNewItem );
+ assert( rItemArr.find(pNewItem) == rItemArr.end() );
+ rItemArr.insert( pNewItem );
return *pNewItem;
}
@@ -704,11 +694,10 @@ void SfxItemPool::Remove( const SfxPoolItem& rItem )
return;
// Find Item in own Pool
- SfxPoolItemArray_Impl* pItemArr = pImpl->maPoolItems[nIndex].get();
- assert(pItemArr && "removing Item not in Pool");
+ SfxPoolItemArray_Impl& rItemArr = pImpl->maPoolItemArrays[nIndex];
- auto it = pItemArr->find(const_cast<SfxPoolItem *>(&rItem));
- if (it != pItemArr->end())
+ auto it = rItemArr.find(const_cast<SfxPoolItem *>(&rItem));
+ if (it != rItemArr.end())
{
if ( rItem.GetRefCount() ) //!
ReleaseRef( rItem );
@@ -722,7 +711,7 @@ void SfxItemPool::Remove( const SfxPoolItem& rItem )
if ( 0 == rItem.GetRefCount() && nWhich < 4000 )
{
delete &rItem;
- pItemArr->erase(it);
+ rItemArr.erase(it);
}
return;
@@ -822,11 +811,8 @@ SfxItemPool::Item2Range
SfxItemPool::GetItemSurrogates(sal_uInt16 nWhich) const
return { EMPTY.end(), EMPTY.end() };
}
- SfxPoolItemArray_Impl* pItemArr =
pImpl->maPoolItems[GetIndex_Impl(nWhich)].get();
- if( pItemArr )
- return { pItemArr->begin(), pItemArr->end() };
-
- return { EMPTY.end(), EMPTY.end() };
+ SfxPoolItemArray_Impl& rItemArr =
pImpl->maPoolItemArrays[GetIndex_Impl(nWhich)];
+ return { rItemArr.begin(), rItemArr.end() };
}
sal_uInt32 SfxItemPool::GetItemCount2(sal_uInt16 nWhich) const
@@ -839,10 +825,8 @@ sal_uInt32 SfxItemPool::GetItemCount2(sal_uInt16 nWhich)
const
return 0;
}
- SfxPoolItemArray_Impl* pItemArr =
pImpl->maPoolItems[GetIndex_Impl(nWhich)].get();
- if ( pItemArr )
- return pItemArr->size();
- return 0;
+ SfxPoolItemArray_Impl& rItemArr =
pImpl->maPoolItemArrays[GetIndex_Impl(nWhich)];
+ return rItemArr.size();
}
@@ -912,10 +896,9 @@ sal_uInt16 SfxItemPool::GetTrueSlotId( sal_uInt16 nWhich )
const
void SfxItemPool::dumpAsXml(xmlTextWriterPtr pWriter) const
{
xmlTextWriterStartElement(pWriter, BAD_CAST("SfxItemPool"));
- for (auto const & rArrayPtr : pImpl->maPoolItems)
- if (rArrayPtr)
- for (auto const & rItem : *rArrayPtr)
- rItem->dumpAsXml(pWriter);
+ for (auto const & rArray : pImpl->maPoolItemArrays)
+ for (auto const & rItem : rArray)
+ rItem->dumpAsXml(pWriter);
xmlTextWriterEndElement(pWriter);
}
diff --git a/svl/source/items/poolio.cxx b/svl/source/items/poolio.cxx
index 569f61c07d24..478fb821d60d 100644
--- a/svl/source/items/poolio.cxx
+++ b/svl/source/items/poolio.cxx
@@ -83,10 +83,9 @@ bool SfxItemPool::CheckItemInPool(const SfxPoolItem *pItem)
const
if( IsStaticDefaultItem(pItem) || IsPoolDefaultItem(pItem) )
return true;
- SfxPoolItemArray_Impl* pItemArr =
pImpl->maPoolItems[GetIndex_Impl(pItem->Which())].get();
- DBG_ASSERT(pItemArr, "ItemArr is not available");
+ SfxPoolItemArray_Impl& rItemArr =
pImpl->maPoolItemArrays[GetIndex_Impl(pItem->Which())];
- for ( auto p : *pItemArr )
+ for ( auto p : rItemArr )
{
if ( p == pItem )
return true;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
