[Libreoffice-commits] core.git: include/svl svl/source

[Noel Grandin (via logerrit)]

 include/svl/itemset.hxx      |    2 +
 svl/source/items/itemset.cxx |   64 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 62 insertions(+), 4 deletions(-)

New commits:
commit 503ab1ca9ae11978d9717557546c01ff598aaf88
Author:     Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sat Oct 2 16:28:56 2021 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Sun Oct 3 20:59:39 2021 +0200

    Use placement new to avoid one of the allocation calls...
    
    ...when cloning a SfxItemSet.
    
    Change-Id: I344fee3863006066eade16db9df37599fc210be3
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123001
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx
index 5ce13bb1f4c8..5f9da72e3339 100644
--- a/include/svl/itemset.hxx
+++ b/include/svl/itemset.hxx
@@ -70,6 +70,8 @@ protected:
     SfxItemSet( SfxItemPool&, SfxAllItemSetFlag );
     /** special constructor for SfxItemSetFixed */
     SfxItemSet( SfxItemPool&, WhichRangesContainer&& ranges, SfxPoolItem const ** ppItems );
+    /** special constructor for Clone */
+    SfxItemSet( const SfxItemSet&, SfxPoolItem const ** ppItems );
 
 public:
     SfxItemSet( const SfxItemSet& );
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 04fab3c9306e..bd63de62f586 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -136,6 +136,44 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
     assert(svl::detail::validRanges2(m_pWhichRanges));
 }
 
+/** special constructor for Clone */
+SfxItemSet::SfxItemSet( const SfxItemSet& rASet, SfxPoolItem const ** ppItems )
+    : m_pPool( rASet.m_pPool )
+    , m_pParent( rASet.m_pParent )
+    , m_ppItems( ppItems )
+    , m_pWhichRanges( rASet.m_pWhichRanges )
+    , m_nCount( rASet.m_nCount )
+    , m_bItemsFixed(true)
+{
+    if (rASet.m_pWhichRanges.empty())
+        return;
+
+    auto nCnt = svl::detail::CountRanges(m_pWhichRanges);
+
+    // Copy attributes
+    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?
+             IsStaticDefaultItem(*ppSrc) )  // Defaults that are not to be pooled?
+            // Just copy the pointer
+            *ppDst = *ppSrc;
+        else if (m_pPool->IsItemPoolable( **ppSrc ))
+        {
+            // Just copy the pointer and increase RefCount
+            *ppDst = *ppSrc;
+            (*ppDst)->AddRef();
+        }
+        else if ( !(*ppSrc)->Which() )
+            *ppDst = (*ppSrc)->Clone();
+        else
+            // !IsPoolable() => assign via Pool
+            *ppDst = &m_pPool->Put( **ppSrc );
+
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+}
+
 SfxItemSet::SfxItemSet(SfxItemSet&& rASet) noexcept
     : m_pPool( rASet.m_pPool )
     , m_pParent( rASet.m_pParent )
@@ -1226,9 +1264,20 @@ bool SfxItemSet::Equals(const SfxItemSet &rCmp, bool bComparePool) const
 
 std::unique_ptr<SfxItemSet> SfxItemSet::Clone(bool bItems, SfxItemPool *pToPool ) const
 {
+    // Use placement new to avoid one of the allocation calls when constructing a new SfxItemSet.
+    // This is effectively a run-time equivalent of the SfxItemSetFixed template.
+    const int cnt = TotalCount();
+    char* p = new char[sizeof(SfxItemSet) + (sizeof(const SfxPoolItem*) * cnt)];
+    SfxItemSet* pNewItemSet = reinterpret_cast<SfxItemSet*>(p);
+    const SfxPoolItem** ppNewItems = reinterpret_cast<const SfxPoolItem **>(p + sizeof(SfxItemSet));
     if (pToPool && pToPool != m_pPool)
     {
-        std::unique_ptr<SfxItemSet> pNewSet(new SfxItemSet(*pToPool, m_pWhichRanges));
+        std::fill(ppNewItems, ppNewItems + cnt, nullptr);
+        std::unique_ptr<SfxItemSet> pNewSet(
+            new (pNewItemSet)
+                SfxItemSet(*pToPool,
+                    WhichRangesContainer(m_pWhichRanges),
+                    ppNewItems));
         if ( bItems )
         {
             SfxWhichIter aIter(*pNewSet);
@@ -1243,10 +1292,17 @@ std::unique_ptr<SfxItemSet> SfxItemSet::Clone(bool bItems, SfxItemPool *pToPool
         }
         return pNewSet;
     }
+    else if (bItems)
+    {
+        return std::unique_ptr<SfxItemSet>(
+            new (pNewItemSet) SfxItemSet(*this, ppNewItems));
+    }
     else
-        return std::unique_ptr<SfxItemSet>(bItems
-                ? new SfxItemSet(*this)
-                : new SfxItemSet(*m_pPool, m_pWhichRanges));
+    {
+        std::fill(ppNewItems, ppNewItems + cnt, nullptr);
+        return std::unique_ptr<SfxItemSet>(
+            new (pNewItemSet) SfxItemSet(*m_pPool, WhichRangesContainer(m_pWhichRanges), ppNewItems));
+    }
 }
 
 SfxItemSet SfxItemSet::CloneAsValue(bool bItems, SfxItemPool *pToPool ) const