tdf#161875 buffer NameOrIndex Items for fast Surrogates

Problem is that collecting the Items using the ItemSets and
PoolItemHolders is too expensive when used too often. For
read-only access it is okay to have the Items directly
registerd (for write access we *need* the ItemSets and
PoolItemHolders, see iterateItemSurrogates).
This is limited to Items which need to support surrogates,
but can further be limited to the here critical ones - the
ones derived from NameOrIndex.
This is done here by checking if the Item is a NameOrIndex
based one by adding a bool to the item that gets set in the
NameOrIndex constructor. If needed this can be changed,
e.g. by using besides the SFX_ITEMINFOFLAG_SUPPORT_SURROGATE
another flag signaling this.
Since only Items that are currently held by an ItemSet or a
PoolItemHolder get registered it is not necessary to change
the Item's RefCount in any way, doing that may lead (again,
we had that with directly set Items at the Pool in the past)
to long-living Items that only get cleaned-up when the pool/
document gets destructed.
This buffering is now SfxItemType-based, no longer using the
WhichID, so the result needs to be checked for WhichID
additionally - if needed (?).
All in all it's anyways a compromize, every usage of the
surrogate mechanism is a 'hack' in the sense that for lazy
reasons not the model data is traversed directly, but assumed
that all Items set at a pool/model *are* model/document data
(what is not always true).

CheckNamedItem does not need to be static, changed that.
Also all accesses to maRegisteredNameOrIndex *have* to
work on the MasterPool instance, same as buffered ItemSets
or PoolItemHolders, corrected that, too.

Number of instances in the buffer need to be counted, else
an instance will be removed too early: The same instance
of an Item can be referenced by multiple sets/holders,
so the first remove from the buffer would already remove
even when the Item is referenced multiple times. Added
that.

Added more asserts & made sure that all constructors/
destructors of SfxItemSet do take care of registering
Items for the surrogate mechanism as needed.

Change-Id: Ib33e7f0bd4abd32a3bb68278f33b0abb9a4754c3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171084
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins

3 files changed, 58 insertions, 4 deletions

diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index 54a348251944..f624eb0aded9 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -402,6 +402,8 @@ void SfxItemPool::registerPoolItemHolder(SfxPoolItemHolder& rHolder)
         SAL_WARN("svl.items", "SfxItemPool::registerPoolItemHolder: SfxPoolItemHolder was already registered (!)");
     }
 #endif
+    if (rHolder.is() && rHolder.getItem()->isNameOrIndex())
+        registerNameOrIndex(*rHolder.getItem());
 }
 
 void SfxItemPool::unregisterPoolItemHolder(SfxPoolItemHolder& rHolder)
@@ -418,6 +420,31 @@ void SfxItemPool::unregisterPoolItemHolder(SfxPoolItemHolder& rHolder)
         SAL_WARN("svl.items", "SfxItemPool::unregisterPoolItemHolder: SfxPoolItemHolder was not registered (!)");
     }
 #endif
+    if (rHolder.is() && rHolder.getItem()->isNameOrIndex())
+        unregisterNameOrIndex(*rHolder.getItem());
+}
+
+void SfxItemPool::registerNameOrIndex(const SfxPoolItem& rItem)
+{
+    assert(rItem.isNameOrIndex() && "ITEM: only Items derived from NameOrIndex supported for this mechanism (!)");
+    NameOrIndexContent& rTarget(GetMasterPool()->maRegisteredNameOrIndex[rItem.ItemType()]);
+    NameOrIndexContent::iterator aHit(rTarget.find(&rItem));
+    if (aHit == rTarget.end())
+        rTarget.insert(std::pair<const SfxPoolItem*, sal_uInt32>(&rItem, 0));
+    else
+        aHit->second++;
+}
+
+void SfxItemPool::unregisterNameOrIndex(const SfxPoolItem& rItem)
+{
+    assert(rItem.isNameOrIndex() && "ITEM: only Items derived from NameOrIndex supported for this mechanism (!)");
+    NameOrIndexContent& rTarget(GetMasterPool()->maRegisteredNameOrIndex[rItem.ItemType()]);
+    NameOrIndexContent::iterator aHit(rTarget.find(&rItem));
+    assert(aHit != rTarget.end() && "ITEM: malformed order of buffered NameOrIndex Items, entry *expected* (!)");
+    if (0 == aHit->second)
+        rTarget.erase(aHit);
+    else
+        aHit->second--;
 }
 
 SfxItemPool* SfxItemPool::getTargetPool(sal_uInt16 nWhich) const
@@ -484,6 +511,7 @@ SfxItemPool::SfxItemPool(const OUString& rName) /* Pool name to identify in the
 , eDefMetric(MapUnit::MapCM)
 , maRegisteredSfxItemSets()
 , maRegisteredSfxPoolItemHolders()
+, maRegisteredNameOrIndex()
 , mbShutdownHintSent(false)
 , maItemInfos()
 , maUserItemInfos()
@@ -509,6 +537,7 @@ SfxItemPool::SfxItemPool(const SfxItemPool& rPool) //  Copy from this instance
 , eDefMetric(MapUnit::MapCM)
 , maRegisteredSfxItemSets()
 , maRegisteredSfxPoolItemHolders()
+, maRegisteredNameOrIndex()
 , mbShutdownHintSent(false)
 , maItemInfos(rPool.maItemInfos)
 , maUserItemInfos(rPool.maUserItemInfos)
@@ -853,6 +882,21 @@ void SfxItemPool::iterateItemSurrogates(
     }
 }
 
+void SfxItemPool::GetItemSurrogatesForItem(ItemSurrogates& rTarget, const SfxPoolItem& rItem) const
+{
+    assert(rItem.isNameOrIndex() && "ITEM: only Items derived from NameOrIndex supported for this mechanism (!)");
+    const registeredNameOrIndex& rRegistered(GetMasterPool()->maRegisteredNameOrIndex);
+    registeredNameOrIndex::const_iterator aHit(rRegistered.find(rItem.ItemType()));
+    if (aHit != rRegistered.end())
+    {
+        rTarget.clear();
+        rTarget.reserve(aHit->second.size());
+        for (const auto& entry : aHit->second)
+            rTarget.push_back(entry.first);
+    }
+        // rTarget = ItemSurrogates(aHit->second.begin(), aHit->second.end());
+}
+
 void SfxItemPool::GetItemSurrogates(ItemSurrogates& rTarget, sal_uInt16 nWhich) const
 {
     rTarget.clear();
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 523ffc857509..8c7e772a1efd 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -256,7 +256,7 @@ SfxItemSet::SfxItemSet(SfxItemPool& pool, WhichRangesContainer wids)
 SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
 : m_pPool( rASet.m_pPool )
 , m_pParent( rASet.m_pParent )
-, m_nRegister( rASet.m_nRegister )
+, m_nRegister( 0 )
 #ifdef DBG_UTIL
 , m_nRegisteredSfxItemIter(0)
 #endif
@@ -271,11 +271,14 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
         return;
 
     for (const auto& rSource : rASet.m_aPoolItemMap)
-        m_aPoolItemMap[rSource.first] = implCreateItemEntry(*GetPool(), rSource.second, false);
+    {
+        const SfxPoolItem* pNew(implCreateItemEntry(*GetPool(), rSource.second, false));
+        m_aPoolItemMap[rSource.first] = pNew;
+        if (m_nRegister != rASet.m_nRegister)
+            checkAddPoolRegistration(pNew);
+    }
 
     assert(m_aWhichRanges.validRanges2());
-    if (0 != m_nRegister)
-        GetPool()->registerItemSet(*this);
 }
 
 SfxItemSet::SfxItemSet(SfxItemSet&& rASet) noexcept
@@ -397,6 +400,9 @@ void SfxItemSet::checkRemovePoolRegistration(const SfxPoolItem* pItem)
     // deregister when no more Items that NeedsSurrogateSupport exist
     if (0 == m_nRegister)
         GetPool()->unregisterItemSet(*this);
+
+    if (pItem->isNameOrIndex())
+        GetPool()->unregisterNameOrIndex(*pItem);
 }
 
 void SfxItemSet::checkAddPoolRegistration(const SfxPoolItem* pItem)
@@ -421,6 +427,9 @@ void SfxItemSet::checkAddPoolRegistration(const SfxPoolItem* pItem)
     if (0 == m_nRegister)
         GetPool()->registerItemSet(*this);
 
+    if (pItem->isNameOrIndex())
+        GetPool()->registerNameOrIndex(*pItem);
+
     // increment counter
     m_nRegister++;
 }
diff --git a/svl/source/items/poolitem.cxx b/svl/source/items/poolitem.cxx
index 85aa8b998878..dd628225a7d8 100644
--- a/svl/source/items/poolitem.cxx
+++ b/svl/source/items/poolitem.cxx
@@ -526,6 +526,7 @@ SfxPoolItem::SfxPoolItem(sal_uInt16 const nWhich, SfxItemType eType)
     , m_bDynamicDefault(false)
     , m_bIsSetItem(false)
     , m_bShareable(true)
+    , m_bNameOrIndex(false)
 #ifdef DBG_UTIL
     , m_bDeleted(false)
 #endif