introduce SfxItemSetFixed and use it in DefaultProperties

DefaultProperties::SetObjectItemSet is very hot when loading
shapes, and a large chunk of that cost is allocating the pool item
array.
So use a template class to allocate the array in-line to the class,
which means it can be allocated on-stack.

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

7 files changed, 92 insertions, 43 deletions

diff --git a/include/svl/itemiter.hxx b/include/svl/itemiter.hxx
index c6d82e41b621..910b430e4161 100644
--- a/include/svl/itemiter.hxx
+++ b/include/svl/itemiter.hxx
@@ -38,7 +38,7 @@ public:
     /// get item, or null if no items
     const SfxPoolItem* GetCurItem() const
     {
-        return m_rSet.m_nCount ? *(m_rSet.m_pItems.get() + m_nCurrent) : nullptr;
+        return m_rSet.m_nCount ? *(m_rSet.m_ppItems + m_nCurrent) : nullptr;
     }
     const SfxPoolItem* NextItem() { return (m_nCurrent < m_nEnd) ? ImplNextItem() : nullptr; }
 
diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx
index 760fafd03992..f0ea42835073 100644
--- a/include/svl/itemset.hxx
+++ b/include/svl/itemset.hxx
@@ -16,8 +16,7 @@
  *   except in compliance with the License. You may obtain a copy of
  *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
  */
-#ifndef INCLUDED_SVL_ITEMSET_HXX
-#define INCLUDED_SVL_ITEMSET_HXX
+#pragma once
 
 #include <sal/config.h>
 
@@ -39,10 +38,10 @@ class SAL_WARN_UNUSED SVL_DLLPUBLIC SfxItemSet
 
     SfxItemPool*      m_pPool;         ///< pool that stores the items
     const SfxItemSet* m_pParent;       ///< derivation
-    std::unique_ptr<SfxPoolItem const*[]>
-                      m_pItems;        ///< array of items
+    SfxPoolItem const** m_ppItems;     ///< pointer to array of items, we allocate and free this unless m_bItemsFixed==true
     WhichRangesContainer m_pWhichRanges;  ///< array of Which Ranges
     sal_uInt16        m_nCount;        ///< number of items
+    bool              m_bItemsFixed; ///< true if this is a SfxItemSetFixed object
 
 friend class SfxItemPoolCache;
 friend class SfxAllItemSet;
@@ -53,7 +52,7 @@ private:
     SfxItemSet( SfxItemPool & pool, const WhichRangesContainer& wids, std::size_t items );
 
 public:
-    SfxPoolItem const**         GetItems_Impl() const { return m_pItems.get(); }
+    SfxPoolItem const**         GetItems_Impl() const { return m_ppItems; }
 
 private:
     const SfxItemSet&           operator=(const SfxItemSet &) = delete;
@@ -69,6 +68,8 @@ protected:
     /** special constructor for SfxAllItemSet */
     enum class SfxAllItemSetFlag { Flag };
     SfxItemSet( SfxItemPool&, SfxAllItemSetFlag );
+    /** special constructor for SfxItemSetFixed */
+    SfxItemSet( SfxItemPool&, WhichRangesContainer&& ranges, SfxPoolItem const ** ppItems );
 
 public:
     SfxItemSet( const SfxItemSet& );
@@ -223,6 +224,17 @@ private:
     virtual const SfxPoolItem*  PutImpl( const SfxPoolItem&, sal_uInt16 nWhich, bool bPassingOwnership ) override;
 };
 
-#endif // INCLUDED_SVL_ITEMSET_HXX
+
+// Allocate the items array inside the object, to reduce allocation cost.
+//
+template<sal_uInt16 nWID1, sal_uInt16 nWID2>
+class SfxItemSetFixed : public SfxItemSet
+{
+public:
+    SfxItemSetFixed( SfxItemPool& rPool)
+        : SfxItemSet(rPool, WhichRangesContainer(svl::Items_t<nWID1, nWID2>{}), m_aItems) {}
+private:
+    const SfxPoolItem* m_aItems[nWID2 - nWID1 + 1] = {};
+};
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/solenv/gdb/libreoffice/svl.py b/solenv/gdb/libreoffice/svl.py
index 28ca2997e5d7..f82b02a7b4a3 100644
--- a/solenv/gdb/libreoffice/svl.py
+++ b/solenv/gdb/libreoffice/svl.py
@@ -39,7 +39,7 @@ class ItemSetPrinter(object):
         for (whichfrom, whichto) in whichranges:
             size += whichto - whichfrom + 1
             whichids += [which for which in range(whichfrom, whichto+1)]
-        return self._iterator(self.value['m_pItems']['_M_t']['_M_t']['_M_head_impl'], size, whichids)
+        return self._iterator(self.value['m_ppItems'], size, whichids)
 
     class _iterator(six.Iterator):
 
diff --git a/solenv/vs/LibreOffice.natvis b/solenv/vs/LibreOffice.natvis
index ee4171bac9cf..0962c1c441fb 100644
--- a/solenv/vs/LibreOffice.natvis
+++ b/solenv/vs/LibreOffice.natvis
@@ -340,7 +340,7 @@
     <DisplayString>{{size={m_nCount,d}}}</DisplayString>
     <Expand>
       <CustomListItems>
-        <Variable Name='pCurItem' InitialValue='m_pItems._Mypair._Myval2'/>
+        <Variable Name='pCurItem' InitialValue='m_ppItems'/>
         <Variable Name='nRanges' InitialValue='m_pWhichRanges.m_size'/>
         <Variable Name='nCurRange' InitialValue='0'/>
         <Variable Name='nCurWhich' InitialValue='0'/>
diff --git a/svl/source/items/itemiter.cxx b/svl/source/items/itemiter.cxx
index fb00339877c7..3e4b927d2af9 100644
--- a/svl/source/items/itemiter.cxx
+++ b/svl/source/items/itemiter.cxx
@@ -30,7 +30,7 @@ SfxItemIter::SfxItemIter(const SfxItemSet& rItemSet)
     }
     else
     {
-        SfxPoolItem const** ppFnd = m_rSet.m_pItems.get();
+        SfxPoolItem const** ppFnd = m_rSet.m_ppItems;
 
         // Find the first Item that is set
         for (m_nStart = 0; !*(ppFnd + m_nStart); ++m_nStart)
@@ -50,7 +50,7 @@ SfxItemIter::~SfxItemIter() {}
 // Precondition : m_nCurrent < m_nEnd
 const SfxPoolItem* SfxItemIter::ImplNextItem()
 {
-    SfxPoolItem const** ppFnd = m_rSet.m_pItems.get();
+    SfxPoolItem const** ppFnd = m_rSet.m_ppItems;
     do
     {
         m_nCurrent++;
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 418471183aed..04fab3c9306e 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -52,18 +52,35 @@ SfxItemSet::SfxItemSet(SfxItemPool& rPool)
 SfxItemSet::SfxItemSet( SfxItemPool& rPool, SfxAllItemSetFlag )
     : m_pPool(&rPool)
     , m_pParent(nullptr)
+    , m_ppItems(nullptr)
     , m_nCount(0)
+    , m_bItemsFixed(false)
 {
 }
 
+/** special constructor for SfxItemSetFixed */
+SfxItemSet::SfxItemSet( SfxItemPool& rPool, WhichRangesContainer&& ranges, SfxPoolItem const ** ppItems )
+    : m_pPool(&rPool)
+    , m_pParent(nullptr)
+    , m_ppItems(ppItems)
+    , m_pWhichRanges(std::move(ranges))
+    , m_nCount(0)
+    , m_bItemsFixed(true)
+{
+    assert(ppItems);
+    assert(m_pWhichRanges.size() > 0);
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+}
+
 SfxItemSet::SfxItemSet(
     SfxItemPool & pool,
     const WhichRangesContainer& wids,
     std::size_t items):
     m_pPool(&pool), m_pParent(nullptr),
-    m_pItems(new SfxPoolItem const *[items]{}),
+    m_ppItems(new SfxPoolItem const *[items]{}),
     m_pWhichRanges(wids),
-    m_nCount(0)
+    m_nCount(0),
+    m_bItemsFixed(false)
 {
     assert(wids.size() != 0);
     assert(svl::detail::validRanges2(m_pWhichRanges));
@@ -84,16 +101,20 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
     , m_pParent( rASet.m_pParent )
     , m_pWhichRanges( rASet.m_pWhichRanges )
     , m_nCount( rASet.m_nCount )
+    , m_bItemsFixed(false)
 {
     if (rASet.m_pWhichRanges.empty())
+    {
+        m_ppItems = nullptr;
         return;
+    }
 
     auto nCnt = svl::detail::CountRanges(m_pWhichRanges);
-    m_pItems.reset(new const SfxPoolItem* [nCnt] {});
+    m_ppItems = new const SfxPoolItem* [nCnt] {};
 
     // Copy attributes
-    SfxPoolItem const** ppDst = m_pItems.get();
-    SfxPoolItem const** ppSrc = rASet.m_pItems.get();
+    SfxPoolItem const** ppDst = m_ppItems;
+    SfxPoolItem const** ppSrc = rASet.m_ppItems;
     for( sal_uInt16 n = nCnt; n; --n, ++ppDst, ++ppSrc )
         if ( nullptr == *ppSrc ||                 // Current Default?
              IsInvalidItem(*ppSrc) ||       // DontCare?
@@ -118,10 +139,21 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
 SfxItemSet::SfxItemSet(SfxItemSet&& rASet) noexcept
     : m_pPool( rASet.m_pPool )
     , m_pParent( rASet.m_pParent )
-    , m_pItems( std::move(rASet.m_pItems) )
+    , m_ppItems( rASet.m_ppItems )
     , m_pWhichRanges( std::move(rASet.m_pWhichRanges) )
     , m_nCount( rASet.m_nCount )
+    , m_bItemsFixed(false)
 {
+    if (rASet.m_bItemsFixed)
+    {
+        // have to make a copy
+        int noItems = svl::detail::CountRanges(m_pWhichRanges);
+        m_ppItems = new const SfxPoolItem* [noItems];
+        std::copy(rASet.m_ppItems, rASet.m_ppItems + noItems, m_ppItems);
+    }
+    else
+        // taking over ownership
+        rASet.m_ppItems = nullptr;
     rASet.m_pPool = nullptr;
     rASet.m_pParent = nullptr;
     rASet.m_nCount = 0;
@@ -135,7 +167,7 @@ SfxItemSet::~SfxItemSet()
         sal_uInt16 nCount = TotalCount();
         if( Count() )
         {
-            SfxPoolItem const** ppFnd = m_pItems.get();
+            SfxPoolItem const** ppFnd = m_ppItems;
             for( sal_uInt16 nCnt = nCount; nCnt; --nCnt, ++ppFnd )
                 if( *ppFnd && !IsInvalidItem(*ppFnd) )
                 {
@@ -154,7 +186,8 @@ SfxItemSet::~SfxItemSet()
         }
     }
 
-    m_pItems.reset();
+    if (!m_bItemsFixed)
+        delete[] m_ppItems;
     m_pWhichRanges.reset(); // for invariant-testing
 }
 
@@ -167,7 +200,7 @@ sal_uInt16 SfxItemSet::ClearItem( sal_uInt16 nWhich )
         return 0;
 
     sal_uInt16 nDel = 0;
-    SfxPoolItem const** ppFnd = m_pItems.get();
+    SfxPoolItem const** ppFnd = m_ppItems;
 
     if( nWhich )
     {
@@ -253,7 +286,7 @@ sal_uInt16 SfxItemSet::ClearItem( sal_uInt16 nWhich )
 
 void SfxItemSet::ClearInvalidItems()
 {
-    SfxPoolItem const** ppFnd = m_pItems.get();
+    SfxPoolItem const** ppFnd = m_ppItems;
     for (const WhichPair& rPair : m_pWhichRanges)
     {
         for( sal_uInt16 nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
@@ -269,7 +302,7 @@ void SfxItemSet::InvalidateAllItems()
 {
     assert( !m_nCount && "There are still Items set" );
     m_nCount = TotalCount();
-    memset(static_cast<void*>(m_pItems.get()), -1, m_nCount * sizeof(SfxPoolItem*));
+    memset(static_cast<void*>(m_ppItems), -1, m_nCount * sizeof(SfxPoolItem*));
 }
 
 SfxItemState SfxItemSet::GetItemState( sal_uInt16 nWhich,
@@ -281,7 +314,7 @@ SfxItemState SfxItemSet::GetItemState( sal_uInt16 nWhich,
     SfxItemState eRet = SfxItemState::UNKNOWN;
     do
     {
-        SfxPoolItem const** ppFnd = pCurrentSet->m_pItems.get();
+        SfxPoolItem const** ppFnd = pCurrentSet->m_ppItems;
         for (const WhichPair& rPair : pCurrentSet->m_pWhichRanges)
         {
             if ( rPair.first <= nWhich && nWhich <= rPair.second )
@@ -334,7 +367,7 @@ const SfxPoolItem* SfxItemSet::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWh
         return nullptr; //FIXME: Only because of Outliner bug
     }
 
-    SfxPoolItem const** ppFnd = m_pItems.get();
+    SfxPoolItem const** ppFnd = m_ppItems;
     for (const WhichPair& rPair : m_pWhichRanges)
     {
         if( rPair.first <= nWhich && nWhich <= rPair.second )
@@ -431,7 +464,7 @@ bool SfxItemSet::Put( const SfxItemSet& rSet, bool bInvalidAsDefault )
     bool bRet = false;
     if( rSet.Count() )
     {
-        SfxPoolItem const** ppFnd = rSet.m_pItems.get();
+        SfxPoolItem const** ppFnd = rSet.m_ppItems;
         for (const WhichPair& rPair : rSet.m_pWhichRanges)
         {
             for ( sal_uInt16 nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
@@ -477,7 +510,7 @@ void SfxItemSet::PutExtended
 )
 {
     // don't "optimize" with "if( rSet.Count()" because of dont-care + defaults
-    SfxPoolItem const** ppFnd = rSet.m_pItems.get();
+    SfxPoolItem const** ppFnd = rSet.m_ppItems;
     for (const WhichPair& rPair : rSet.m_pWhichRanges)
     {
         for ( sal_uInt16 nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
@@ -619,14 +652,18 @@ void SfxItemSet::RecreateRanges_Impl(const WhichRangesContainer& pNewRanges)
         sal_uInt16 nOldTotalCount = TotalCount();
         for ( sal_uInt16 nItem = 0; nItem < nOldTotalCount; ++nItem )
         {
-            const SfxPoolItem *pItem = m_pItems[nItem];
+            const SfxPoolItem *pItem = m_ppItems[nItem];
             if ( pItem && !IsInvalidItem(pItem) && pItem->Which() )
                 m_pPool->Remove(*pItem);
         }
     }
 
     // replace old items-array and ranges
-    m_pItems.reset( aNewItems );
+    if (m_bItemsFixed)
+        m_bItemsFixed = false;
+    else
+        delete[] m_ppItems;
+    m_ppItems = aNewItems;
     m_nCount = nNewCount;
 }
 
@@ -710,7 +747,7 @@ const SfxPoolItem& SfxItemSet::Get( sal_uInt16 nWhich, bool bSrchInParent) const
     {
         if( pCurrentSet->Count() )
         {
-            SfxPoolItem const** ppFnd = pCurrentSet->m_pItems.get();
+            SfxPoolItem const** ppFnd = pCurrentSet->m_ppItems;
             for (auto const & pPtr : pCurrentSet->m_pWhichRanges)
             {
                 if( pPtr.first <= nWhich && nWhich <= pPtr.second )
@@ -784,8 +821,8 @@ void SfxItemSet::Intersect( const SfxItemSet& rSet )
     if( m_pWhichRanges == rSet.m_pWhichRanges )
     {
         sal_uInt16 nSize = TotalCount();
-        SfxPoolItem const** ppFnd1 = m_pItems.get();
-        SfxPoolItem const** ppFnd2 = rSet.m_pItems.get();
+        SfxPoolItem const** ppFnd1 = m_ppItems;
+        SfxPoolItem const** ppFnd2 = rSet.m_ppItems;
 
         for( ; nSize; --nSize, ++ppFnd1, ++ppFnd2 )
             if( *ppFnd1 && !*ppFnd2 )
@@ -833,8 +870,8 @@ void SfxItemSet::Differentiate( const SfxItemSet& rSet )
     if( m_pWhichRanges == rSet.m_pWhichRanges )
     {
         sal_uInt16 nSize = TotalCount();
-        SfxPoolItem const** ppFnd1 = m_pItems.get();
-        SfxPoolItem const** ppFnd2 = rSet.m_pItems.get();
+        SfxPoolItem const** ppFnd1 = m_ppItems;
+        SfxPoolItem const** ppFnd2 = rSet.m_ppItems;
 
         for( ; nSize; --nSize, ++ppFnd1, ++ppFnd2 )
             if( *ppFnd1 && *ppFnd2 )
@@ -1026,8 +1063,8 @@ void SfxItemSet::MergeValues( const SfxItemSet& rSet )
     if( m_pWhichRanges == rSet.m_pWhichRanges )
     {
         sal_uInt16 nSize = TotalCount();
-        SfxPoolItem const** ppFnd1 = m_pItems.get();
-        SfxPoolItem const** ppFnd2 = rSet.m_pItems.get();
+        SfxPoolItem const** ppFnd1 = m_ppItems;
+        SfxPoolItem const** ppFnd2 = rSet.m_ppItems;
 
         for( ; nSize; --nSize, ++ppFnd1, ++ppFnd2 )
             MergeItem_Impl(m_pPool, m_nCount, ppFnd1, *ppFnd2, false/*bIgnoreDefaults*/);
@@ -1056,7 +1093,7 @@ void SfxItemSet::MergeValues( const SfxItemSet& rSet )
 
 void SfxItemSet::MergeValue( const SfxPoolItem& rAttr, bool bIgnoreDefaults )
 {
-    SfxPoolItem const** ppFnd = m_pItems.get();
+    SfxPoolItem const** ppFnd = m_ppItems;
     const sal_uInt16 nWhich = rAttr.Which();
     for( auto const & pPtr : m_pWhichRanges )
     {
@@ -1073,7 +1110,7 @@ void SfxItemSet::MergeValue( const SfxPoolItem& rAttr, bool bIgnoreDefaults )
 
 void SfxItemSet::InvalidateItem( sal_uInt16 nWhich )
 {
-    SfxPoolItem const** ppFnd = m_pItems.get();
+    SfxPoolItem const** ppFnd = m_ppItems;
     for( auto const & pPtr : m_pWhichRanges )
     {
         if( pPtr.first <= nWhich && nWhich <= pPtr.second )
@@ -1163,12 +1200,12 @@ bool SfxItemSet::Equals(const SfxItemSet &rCmp, bool bComparePool) const
     }
 
     // Are all pointers the same?
-    if (0 == memcmp( m_pItems.get(), rCmp.m_pItems.get(), nCount1 * sizeof(m_pItems[0]) ))
+    if (0 == memcmp( m_ppItems, rCmp.m_ppItems, nCount1 * sizeof(m_ppItems[0]) ))
         return true;
 
     // We need to compare each one separately then
-    const SfxPoolItem **ppItem1 = m_pItems.get();
-    const SfxPoolItem **ppItem2 = rCmp.m_pItems.get();
+    const SfxPoolItem **ppItem1 = m_ppItems;
+    const SfxPoolItem **ppItem2 = rCmp.m_ppItems;
     for ( sal_uInt16 nPos = 0; nPos < nCount1; ++nPos )
     {
         // If the pointers of the poolable Items are not the same, the Items
@@ -1239,7 +1276,7 @@ SfxItemSet SfxItemSet::CloneAsValue(bool bItems, SfxItemPool *pToPool ) const
 
 void SfxItemSet::PutDirect(const SfxPoolItem &rItem)
 {
-    SfxPoolItem const** ppFnd = m_pItems.get();
+    SfxPoolItem const** ppFnd = m_ppItems;
     const sal_uInt16 nWhich = rItem.Which();
 #ifdef DBG_UTIL
     IsPoolDefaultItem(&rItem) || m_pPool->CheckItemInPool(&rItem);
diff --git a/svx/source/sdr/properties/defaultproperties.cxx b/svx/source/sdr/properties/defaultproperties.cxx
index cd6a00a0981c..fd27a9744d80 100644
--- a/svx/source/sdr/properties/defaultproperties.cxx
+++ b/svx/source/sdr/properties/defaultproperties.cxx
@@ -186,9 +186,9 @@ namespace sdr::properties
             const SfxPoolItem *pPoolItem;
             std::vector< sal_uInt16 > aPostItemChangeList;
             bool bDidChange(false);
-            std::optional<SfxItemSet> aSet;
+            std::optional<SfxItemSetFixed<SDRATTR_START, EE_ITEMS_END>> aSet;
             if (WantItemSetInItemSetChanged())
-                aSet.emplace(GetSdrObject().GetObjectItemPool(), svl::Items<SDRATTR_START, EE_ITEMS_END>);
+                aSet.emplace(GetSdrObject().GetObjectItemPool());
 
             // give a hint to STL_Vector
             aPostItemChangeList.reserve(rSet.Count());