[Libreoffice-commits] core.git: include/svl solenv/gdb solenv/vs svl/source svx/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Mon Sep 20 15:48:42 UTC 2021
include/svl/itemiter.hxx | 2
include/svl/itemset.hxx | 24 ++++-
solenv/gdb/libreoffice/svl.py | 2
solenv/vs/LibreOffice.natvis | 2
svl/source/items/itemiter.cxx | 4
svl/source/items/itemset.cxx | 97 ++++++++++++++++--------
svx/source/sdr/properties/defaultproperties.cxx | 4
7 files changed, 92 insertions(+), 43 deletions(-)
New commits:
commit afd918a81bc2dce4830bc94cbd88b9038f5715ff
Author: Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sun Sep 19 11:13:53 2021 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Mon Sep 20 17:48:08 2021 +0200
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 at collabora.co.uk>
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());
More information about the Libreoffice-commits
mailing list