include/svl/poolitem.hxx | 7 +++++++ sc/inc/patattr.hxx | 1 + sc/source/core/data/patattr.cxx | 10 +++++++++- svl/source/items/itempool.cxx | 24 ++++++++++++++++++++---- 4 files changed, 37 insertions(+), 5 deletions(-)
New commits: commit 98d51bf8ce13bdac2d71f50f58d6d0ddb9041a4f Author: Luboš Luňák <[email protected]> AuthorDate: Tue Feb 8 11:36:42 2022 +0100 Commit: Luboš Luňák <[email protected]> CommitDate: Tue Feb 8 20:57:41 2022 +0100 speed up SfxItemPool searches with items that can't use IsSortable() As pointed out by 585e0ac43b9bd8a2f714903034e435c84ae3fc96, some item types cannot be used as IsSortable(), because they are modified after having been inserted in the pool, which breaks the sorted order. But it's possible to at least somewhat improve performance of these items by explicitly providing a hash code and using that first for comparisons when looking up items, which may be cheaper than calling operator==. With ScPatternAttr such comparisons seem to take only 60% of the original time, reducing loading time of some documents by 25%. Change-Id: I41f4dda472fb6db066742976672f2c08b9aeef63 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129667 Reviewed-by: Noel Grandin <[email protected]> Reviewed-by: Luboš Luňák <[email protected]> Tested-by: Jenkins diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx index ba22aed9a9cc..1549cd0a6298 100644 --- a/include/svl/poolitem.hxx +++ b/include/svl/poolitem.hxx @@ -187,6 +187,13 @@ 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; } /** @return true if it has a valid string representation */ virtual bool GetPresentation( SfxItemPresentation ePresentation, diff --git a/sc/inc/patattr.hxx b/sc/inc/patattr.hxx index e6ac9603dd17..f60144c2feaf 100644 --- a/sc/inc/patattr.hxx +++ b/sc/inc/patattr.hxx @@ -66,6 +66,7 @@ public: virtual ScPatternAttr* Clone( SfxItemPool *pPool = nullptr ) const override; virtual bool operator==(const SfxPoolItem& rCmp) const override; + virtual size_t LookupHashCode() 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 f474cb052232..c92c409804e5 100644 --- a/sc/source/core/data/patattr.cxx +++ b/sc/source/core/data/patattr.cxx @@ -154,6 +154,13 @@ bool ScPatternAttr::operator==( const SfxPoolItem& rCmp ) const StrCmp( GetStyleName(), rOther.GetStyleName() ); } +size_t ScPatternAttr::LookupHashCode() const +{ + if (!mxHashCode) + CalcHashCode(); + return *mxHashCode; +} + SvxCellOrientation ScPatternAttr::GetCellOrientation( const SfxItemSet& rItemSet, const SfxItemSet* pCondSet ) { SvxCellOrientation eOrient = SvxCellOrientation::Standard; @@ -1367,7 +1374,8 @@ sal_uInt64 ScPatternAttr::GetKey() const void ScPatternAttr::CalcHashCode() const { auto const & rSet = GetItemSet(); - mxHashCode = boost::hash_range(rSet.GetItems_Impl(), rSet.GetItems_Impl() + rSet.Count()); + mxHashCode = 1; // Set up seed so that an empty pattern does not have an (invalid) hash of 0. + boost::hash_range(*mxHashCode, rSet.GetItems_Impl(), rSet.GetItems_Impl() + rSet.Count()); } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx index 8d4aa074e4f5..acd5c77de452 100644 --- a/svl/source/items/itempool.cxx +++ b/svl/source/items/itempool.cxx @@ -627,12 +627,28 @@ const SfxPoolItem& SfxItemPool::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nW } else { - for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr) + // If the item provides a valid hash, use that to speed up comparisons. + size_t lookupHashCode = rItem.LookupHashCode(); + if (lookupHashCode != 0) { - if (**itr == rItem) + for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr) { - pFoundItem = *itr; - break; + 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; + } } } }
