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.grandin@collabora.co.uk>
diff --git a/svl/qa/unit/items/test_itempool.cxx b/svl/qa/unit/items/test_itempool.cxx
index 9b55285..e08c77d 100644
--- a/svl/qa/unit/items/test_itempool.cxx
+++ b/svl/qa/unit/items/test_itempool.cxx
@@ -43,17 +43,17 @@
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 @@
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 @@
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 eb78be1..8eef10c 100644
--- a/svl/source/inc/poolio.hxx
+++ b/svl/source/inc/poolio.hxx
@@ -43,21 +43,22 @@
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 @@
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 @@
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 3a0babf..b407889 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -250,7 +250,7 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
}
// 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 @@
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 @@
// 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 @@
}
// 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 @@
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 @@
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 @@
if ( 0 == rItem.GetRefCount() && nWhich < 4000 )
{
delete &rItem;
pItemArr->erase(it);
rItemArr.erase(it);
}
return;
@@ -822,11 +811,8 @@
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 @@
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 @@
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 569f61c..478fb82 100644
--- a/svl/source/items/poolio.cxx
+++ b/svl/source/items/poolio.cxx
@@ -83,10 +83,9 @@
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;
