include/svl/poolitem.hxx | 17 ++++++++++++-----
include/svl/setitem.hxx | 1 -
sc/inc/patattr.hxx | 6 +++---
sc/source/core/data/patattr.cxx | 25 ++++++++++++++++++-------
svl/source/items/itempool.cxx | 30 ++++++++++--------------------
svl/source/items/sitem.cxx | 5 -----
6 files changed, 43 insertions(+), 41 deletions(-)
New commits:
commit b26c34267cdf9d0b7ba4e2fda7ae706d5cd76299
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Mon Feb 21 00:57:21 2022 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Mon Feb 21 12:29:49 2022 +0100
replace SfxPoolItem::LookupHashCode() with Lookup() (tdf#135215)
The LookupHashCode() way still loops over an array while calling
virtual functions for a type that is actually known. Overriding
instead a whole new Lookup() function that does the loop
avoids the virtual calls, allowing compiler optimizations such
as inlining, or class-specific optimizations like the hash code.
So this still improves performance with ScPatternAttr a bit,
for tdf#135215 it saves about 40% of load time, but this is
scraping the bottom of the barrel, as this is still a linear search
(the IsSortable() approach, if it worked, would be still 3x faster).
Change-Id: I8fe5f70cabb77e2f6619d169beee8a3b4da46213
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130228
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index 1549cd0a6298..10a7901b4e70 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -23,6 +23,7 @@
#include <sal/config.h>
#include <memory>
+#include <vector>
#include <com/sun/star/uno/Any.hxx>
#include <svl/hint.hxx>
@@ -187,13 +188,19 @@ public:
// of a single kind of pool item.
virtual bool operator<( const SfxPoolItem& ) const {
assert(false); return false; }
virtual bool IsSortable() const { return false; }
+
// Some item types cannot be IsSortable() (such as because they are
modified while stored
// in a pool, which would change the ordering position, see e.g.
585e0ac43b9bd8a2f714903034).
- // To improve performance in such cases it is possible to return a
(fast/cached) non-zero hash code,
- // which will be used to speed up linear lookup and only matching items
will be fully compared.
- // Note: Since 0 value is invalid, try to make sure it's not returned for
valid items
- // (which may easily happen e.g. if the item is empty and the hash is
seeded with 0).
- virtual size_t LookupHashCode() const { return 0; }
+ // To improve performance in such cases it is possible to reimplement
Lookup() to do a linear
+ // lookup optimized for the specific class (avoiding virtual functions may
allow the compiler
+ // to generate better code and class-specific optimizations such as
hashing or caching may
+ // be used.)
+ // If reimplemented, the Lookup() function should search [begin,end) for
an item matching
+ // this object and return an iterator pointing to the item or the end
iterator.
+ virtual bool HasLookup() const { return false; }
+ typedef std::vector<SfxPoolItem*>::const_iterator lookup_iterator;
+ virtual lookup_iterator Lookup(lookup_iterator /*begin*/, lookup_iterator
end ) const
+ { assert( false ); return end; }
/** @return true if it has a valid string representation */
virtual bool GetPresentation( SfxItemPresentation
ePresentation,
diff --git a/include/svl/setitem.hxx b/include/svl/setitem.hxx
index 8a7c39f08ea0..69ebea4b1d23 100644
--- a/include/svl/setitem.hxx
+++ b/include/svl/setitem.hxx
@@ -34,7 +34,6 @@ public:
SfxSetItem(sal_uInt16 nWhich, SfxItemSet&& pSet);
SfxSetItem(sal_uInt16 nWhich, const SfxItemSet& rSet);
SfxSetItem(const SfxSetItem&, SfxItemPool* pPool = nullptr);
- virtual ~SfxSetItem() override;
virtual bool operator==(const SfxPoolItem&) const override;
diff --git a/sc/inc/patattr.hxx b/sc/inc/patattr.hxx
index 261cccc8cf00..4570924d5d17 100644
--- a/sc/inc/patattr.hxx
+++ b/sc/inc/patattr.hxx
@@ -60,12 +60,12 @@ public:
ScPatternAttr(SfxItemPool* pItemPool);
ScPatternAttr(const ScPatternAttr& rPatternAttr);
- virtual ~ScPatternAttr() override;
-
virtual ScPatternAttr* Clone( SfxItemPool *pPool = nullptr ) const
override;
virtual bool operator==(const SfxPoolItem& rCmp) const override;
- virtual size_t LookupHashCode() const override;
+ // Class cannot be IsSortable() because it's mutable, implement at least
Lookup().
+ virtual bool HasLookup() const override { return true; }
+ virtual lookup_iterator Lookup(lookup_iterator begin, lookup_iterator end
) const override;
const SfxPoolItem& GetItem( sal_uInt16 nWhichP ) const
{ return GetItemSet().Get(nWhichP); }
diff --git a/sc/source/core/data/patattr.cxx b/sc/source/core/data/patattr.cxx
index aaab0183fa19..2fa97d3a03dc 100644
--- a/sc/source/core/data/patattr.cxx
+++ b/sc/source/core/data/patattr.cxx
@@ -96,10 +96,6 @@ ScPatternAttr::ScPatternAttr( const ScPatternAttr&
rPatternAttr )
{
}
-ScPatternAttr::~ScPatternAttr()
-{
-}
-
ScPatternAttr* ScPatternAttr::Clone( SfxItemPool *pPool ) const
{
ScPatternAttr* pPattern = new ScPatternAttr(
GetItemSet().CloneAsValue(true, pPool) );
@@ -160,11 +156,26 @@ bool ScPatternAttr::operator==( const SfxPoolItem& rCmp )
const
StrCmp( GetStyleName(), rOther.GetStyleName() );
}
-size_t ScPatternAttr::LookupHashCode() const
+SfxPoolItem::lookup_iterator ScPatternAttr::Lookup(lookup_iterator begin,
lookup_iterator end ) const
{
- if (SAL_UNLIKELY(!mxHashCode))
+ if( !mxHashCode )
CalcHashCode();
- return *mxHashCode;
+ if( *mxHashCode != 0 )
+ {
+ for( auto it = begin; it != end; ++it)
+ {
+ const ScPatternAttr* other = static_cast<const
ScPatternAttr*>(*it);
+ if( !other->mxHashCode )
+ other->CalcHashCode();
+ if (*mxHashCode == *other->mxHashCode
+ && EqualPatternSets( GetItemSet(), other->GetItemSet())
+ && StrCmp( GetStyleName(), other->GetStyleName()))
+ {
+ return it;
+ }
+ }
+ }
+ return end;
}
SvxCellOrientation ScPatternAttr::GetCellOrientation( const SfxItemSet&
rItemSet, const SfxItemSet* pCondSet )
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index acd5c77de452..e13a1ffd60ea 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -625,30 +625,20 @@ const SfxPoolItem& SfxItemPool::PutImpl( const
SfxPoolItem& rItem, sal_uInt16 nW
if (pFoundItem)
assert(*pFoundItem == rItem);
}
+ else if(rItem.HasLookup())
+ {
+ auto it = rItem.Lookup(rItemArr.begin(), rItemArr.end());
+ if( it != rItemArr.end())
+ pFoundItem = *it;
+ }
else
{
- // If the item provides a valid hash, use that to speed up
comparisons.
- size_t lookupHashCode = rItem.LookupHashCode();
- if (lookupHashCode != 0)
+ for (auto it = rItemArr.begin(); it != rItemArr.end(); ++it)
{
- for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr)
+ if (**it == rItem)
{
- if ((*itr)->LookupHashCode() == lookupHashCode && **itr ==
rItem)
- {
- pFoundItem = *itr;
- break;
- }
- }
- }
- else
- {
- for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr)
- {
- if (**itr == rItem)
- {
- pFoundItem = *itr;
- break;
- }
+ pFoundItem = *it;
+ break;
}
}
}
diff --git a/svl/source/items/sitem.cxx b/svl/source/items/sitem.cxx
index c4734d35c2f2..b6ffce978aef 100644
--- a/svl/source/items/sitem.cxx
+++ b/svl/source/items/sitem.cxx
@@ -51,11 +51,6 @@ SfxSetItem::SfxSetItem( const SfxSetItem& rCopy, SfxItemPool
*pPool ) :
}
-SfxSetItem::~SfxSetItem()
-{
-}
-
-
bool SfxSetItem::operator==( const SfxPoolItem& rCmp) const
{
assert(SfxPoolItem::operator==(rCmp));
