[Libreoffice-commits] core.git: include/svl sc/inc sc/source svl/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Sat Apr 20 16:19:07 UTC 2019
include/svl/poolitem.hxx | 5 +++
sc/inc/attrib.hxx | 2 +
sc/inc/patattr.hxx | 2 +
sc/source/core/data/attrib.cxx | 16 +++++++++++
sc/source/core/data/patattr.cxx | 54 +++++++++++++++++++++++++++++++++++++++-
svl/source/inc/poolio.hxx | 30 ++++++++++++++++++++--
svl/source/items/itempool.cxx | 23 +++++++++++++----
svl/source/items/poolio.cxx | 1
8 files changed, 124 insertions(+), 9 deletions(-)
New commits:
commit 003d11f410b7e515981b3efbd65d936d94d87121
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Sat Apr 20 08:32:33 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Sat Apr 20 18:18:20 2019 +0200
tdf#81765 slow loading of .ods with >1000 of conditional formats
This takes the loaing time from 1m38 to 15s.
Speed up finding existing pool items in the pool by storing a
sorted_vector, sorted by a compare operator on the item.
memcmp is faster than operator< on std::vector because operator< seems
to be trying to use some kind of lexicographical compare
Change-Id: Id72527106d604adb8cd2d158bb42f89e2b16a85d
Reviewed-on: https://gerrit.libreoffice.org/71009
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index c1c017c13f44..c3800cc02c18 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -152,6 +152,11 @@ public:
bool operator!=( const SfxPoolItem& rItem ) const
{ return !(*this == rItem); }
+ // Sorting is only used for faster searching in a pool which contains large quantities
+ // of a single kind of pool item.
+ virtual bool operator<( const SfxPoolItem& ) const { assert(false); return false; }
+ virtual bool IsSortable() const { return false; }
+
/** @return true if it has a valid string representation */
virtual bool GetPresentation( SfxItemPresentation ePresentation,
MapUnit eCoreMetric,
diff --git a/sc/inc/attrib.hxx b/sc/inc/attrib.hxx
index 9e4a556080e4..e03d1a223d66 100644
--- a/sc/inc/attrib.hxx
+++ b/sc/inc/attrib.hxx
@@ -269,6 +269,8 @@ public:
virtual ~ScCondFormatItem() override;
virtual bool operator==(const SfxPoolItem& rCmp ) const override;
+ virtual bool operator<(const SfxPoolItem& rCmp) const override;
+ virtual bool IsSortable() const override { return true; }
virtual ScCondFormatItem* Clone( SfxItemPool* = nullptr ) const override;
const std::vector<sal_uInt32>& GetCondFormatData() const { return maIndex;}
diff --git a/sc/inc/patattr.hxx b/sc/inc/patattr.hxx
index 8004a57c3a26..52fd8536d861 100644
--- a/sc/inc/patattr.hxx
+++ b/sc/inc/patattr.hxx
@@ -65,6 +65,8 @@ public:
virtual SfxPoolItem* Clone( SfxItemPool *pPool = nullptr ) const override;
virtual bool operator==(const SfxPoolItem& rCmp) const override;
+ virtual bool operator<(const SfxPoolItem& rCmp) const override;
+ virtual bool IsSortable() const override { return true; }
const SfxPoolItem& GetItem( sal_uInt16 nWhichP ) const
{ return GetItemSet().Get(nWhichP); }
diff --git a/sc/source/core/data/attrib.cxx b/sc/source/core/data/attrib.cxx
index 43cb67ecd91d..37c9c51b612d 100644
--- a/sc/source/core/data/attrib.cxx
+++ b/sc/source/core/data/attrib.cxx
@@ -682,7 +682,21 @@ ScCondFormatItem::~ScCondFormatItem()
bool ScCondFormatItem::operator==( const SfxPoolItem& rCmp ) const
{
- return maIndex == static_cast<const ScCondFormatItem&>(rCmp).maIndex;
+ auto const & other = static_cast<const ScCondFormatItem&>(rCmp);
+ // memcmp is faster than operator< on std::vector
+ return maIndex.size() == other.maIndex.size()
+ && memcmp(maIndex.data(), other.maIndex.data(), maIndex.size() * sizeof(sal_uInt32)) == 0;
+}
+
+bool ScCondFormatItem::operator<( const SfxPoolItem& rCmp ) const
+{
+ auto const & other = static_cast<const ScCondFormatItem&>(rCmp);
+ if ( maIndex.size() < other.maIndex.size() )
+ return true;
+ if ( maIndex.size() > other.maIndex.size() )
+ return false;
+ // memcmp is faster than operator< on std::vector
+ return memcmp(maIndex.data(), other.maIndex.data(), maIndex.size() * sizeof(sal_uInt32)) < 0;
}
ScCondFormatItem* ScCondFormatItem::Clone(SfxItemPool*) const
diff --git a/sc/source/core/data/patattr.cxx b/sc/source/core/data/patattr.cxx
index 4c3c937b4088..bff76e330233 100644
--- a/sc/source/core/data/patattr.cxx
+++ b/sc/source/core/data/patattr.cxx
@@ -111,7 +111,30 @@ SfxPoolItem* ScPatternAttr::Clone( SfxItemPool *pPool ) const
static bool StrCmp( const OUString* pStr1, const OUString* pStr2 )
{
- return ( pStr1 ? ( pStr2 && ( *pStr1 == *pStr2 ) ) : ( pStr2 == nullptr ) );
+ if (pStr1 == pStr2)
+ return true;
+ if (pStr1 && !pStr2)
+ return false;
+ if (!pStr1 && pStr2)
+ return false;
+ // we don't care about a proper lexicographic ordering, we just care about a stable order, and
+ // this is faster
+ return strcmp(reinterpret_cast<const char*>(pStr1->getStr()),
+ reinterpret_cast<const char*>(pStr2->getStr())) == 0;
+}
+
+static bool StrLess( const OUString* pStr1, const OUString* pStr2 )
+{
+ if (pStr1 == pStr2)
+ return false;
+ if (pStr1 && !pStr2)
+ return false;
+ if (!pStr1 && pStr2)
+ return true;
+ // we don't care about a proper lexicographic ordering, we just care about a stable order, and
+ // this is faster
+ return strcmp(reinterpret_cast<const char*>(pStr1->getStr()),
+ reinterpret_cast<const char*>(pStr2->getStr())) < 0;
}
static bool EqualPatternSets( const SfxItemSet& rSet1, const SfxItemSet& rSet2 )
@@ -129,6 +152,23 @@ static bool EqualPatternSets( const SfxItemSet& rSet1, const SfxItemSet& rSet2 )
return ( 0 == memcmp( pItems1, pItems2, (ATTR_PATTERN_END - ATTR_PATTERN_START + 1) * sizeof(pItems1[0]) ) );
}
+static int CmpPatternSets( const SfxItemSet& rSet1, const SfxItemSet& rSet2 )
+{
+ // #i62090# The SfxItemSet in the SfxSetItem base class always has the same ranges
+ // (single range from ATTR_PATTERN_START to ATTR_PATTERN_END), and the items are pooled,
+ // so it's enough to compare just the pointers (Count just because it's even faster).
+
+ if ( rSet1.Count() < rSet2.Count() )
+ return -1;
+ if ( rSet1.Count() > rSet2.Count() )
+ return 1;
+
+ SfxPoolItem const ** pItems1 = rSet1.GetItems_Impl(); // inline method of SfxItemSet
+ SfxPoolItem const ** pItems2 = rSet2.GetItems_Impl();
+
+ return memcmp( pItems1, pItems2, (ATTR_PATTERN_END - ATTR_PATTERN_START + 1) * sizeof(pItems1[0]) );
+}
+
bool ScPatternAttr::operator==( const SfxPoolItem& rCmp ) const
{
// #i62090# Use quick comparison between ScPatternAttr's ItemSets
@@ -137,6 +177,18 @@ bool ScPatternAttr::operator==( const SfxPoolItem& rCmp ) const
StrCmp( GetStyleName(), static_cast<const ScPatternAttr&>(rCmp).GetStyleName() ) );
}
+bool ScPatternAttr::operator<( const SfxPoolItem& rCmp ) const
+{
+ // #i62090# Use quick comparison between ScPatternAttr's ItemSets
+ auto const & rOtherAttr = static_cast<const ScPatternAttr&>(rCmp);
+ int cmp = CmpPatternSets( GetItemSet(), rOtherAttr.GetItemSet() );
+ if (cmp < 0)
+ return true;
+ if (cmp > 0)
+ return false;
+ return StrLess(GetStyleName(), rOtherAttr.GetStyleName());
+}
+
SvxCellOrientation ScPatternAttr::GetCellOrientation( const SfxItemSet& rItemSet, const SfxItemSet* pCondSet )
{
SvxCellOrientation eOrient = SvxCellOrientation::Standard;
diff --git a/svl/source/inc/poolio.hxx b/svl/source/inc/poolio.hxx
index 8eef10c5af96..805406365284 100644
--- a/svl/source/inc/poolio.hxx
+++ b/svl/source/inc/poolio.hxx
@@ -20,6 +20,7 @@
#ifndef INCLUDED_SVL_SOURCE_INC_POOLIO_HXX
#define INCLUDED_SVL_SOURCE_INC_POOLIO_HXX
+#include <sal/log.hxx>
#include <svl/itempool.hxx>
#include <svl/SfxBroadcaster.hxx>
#include <tools/debug.hxx>
@@ -32,6 +33,13 @@ class SfxItemPoolUser;
static const sal_uInt32 SFX_ITEMS_DEFAULT = 0xfffffffe;
+struct CompareSortablePoolItems
+{
+ bool operator()(SfxPoolItem const* lhs, SfxPoolItem const* rhs) const
+ {
+ return (*lhs) < (*rhs);
+ }
+};
/**
* This array contains a set of SfxPoolItems, if those items are
* poolable then each item has a unique set of properties, and we
@@ -42,6 +50,7 @@ struct SfxPoolItemArray_Impl
{
private:
o3tl::sorted_vector<SfxPoolItem*> maPoolItemSet;
+ o3tl::sorted_vector<const SfxPoolItem*, CompareSortablePoolItems> maSortablePoolItems;
public:
o3tl::sorted_vector<SfxPoolItem*>::const_iterator begin() const { return maPoolItemSet.begin(); }
o3tl::sorted_vector<SfxPoolItem*>::const_iterator end() const { return maPoolItemSet.end(); }
@@ -50,9 +59,26 @@ public:
void clear();
size_t size() const {return maPoolItemSet.size();}
bool empty() const {return maPoolItemSet.empty();}
- void insert(SfxPoolItem* pItem) { maPoolItemSet.insert(pItem); }
+ void insert(SfxPoolItem* pItem)
+ {
+ maPoolItemSet.insert(pItem);
+ if (pItem->IsSortable())
+ maSortablePoolItems.insert(pItem);
+ else
+ SAL_WARN_IF(maPoolItemSet.size() > 1024, "svl.items", "make this item sortable to speed up managing this set");
+ }
o3tl::sorted_vector<SfxPoolItem*>::const_iterator find(SfxPoolItem* pItem) const { return maPoolItemSet.find(pItem); }
- void erase(o3tl::sorted_vector<SfxPoolItem*>::const_iterator it) { return maPoolItemSet.erase(it); }
+ const SfxPoolItem* findByLessThan(const SfxPoolItem* pItem) const
+ {
+ auto it = maSortablePoolItems.find(pItem);
+ return it == maSortablePoolItems.end() ? nullptr : *it;
+ }
+ void erase(o3tl::sorted_vector<SfxPoolItem*>::const_iterator it)
+ {
+ if ((*it)->IsSortable())
+ maSortablePoolItems.erase(*it);
+ return maPoolItemSet.erase(it);
+ }
};
struct SfxItemPool_Impl
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index b407889db8e0..67f141dbd16b 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -628,12 +628,25 @@ const SfxPoolItem& SfxItemPool::Put( const SfxPoolItem& rItem, sal_uInt16 nWhich
}
// 2. search for an item with matching attributes.
- for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr)
+ if (rItem.IsSortable())
{
- if (**itr == rItem)
+ auto pFoundItem = rItemArr.findByLessThan(&rItem);
+ if (pFoundItem)
{
- AddRef(**itr);
- return **itr;
+ assert(*pFoundItem == rItem);
+ AddRef(*pFoundItem);
+ return *pFoundItem;
+ }
+ }
+ else
+ {
+ for (auto itr = rItemArr.begin(); itr != rItemArr.end(); ++itr)
+ {
+ if (**itr == rItem)
+ {
+ AddRef(**itr);
+ return **itr;
+ }
}
}
}
@@ -710,8 +723,8 @@ void SfxItemPool::Remove( const SfxPoolItem& rItem )
// See other MI-REF
if ( 0 == rItem.GetRefCount() && nWhich < 4000 )
{
- delete &rItem;
rItemArr.erase(it);
+ delete &rItem;
}
return;
diff --git a/svl/source/items/poolio.cxx b/svl/source/items/poolio.cxx
index 478fb821d60d..a909450f4588 100644
--- a/svl/source/items/poolio.cxx
+++ b/svl/source/items/poolio.cxx
@@ -33,6 +33,7 @@
void SfxPoolItemArray_Impl::clear()
{
maPoolItemSet.clear();
+ maSortablePoolItems.clear();
}
sal_uInt16 SfxItemPool::GetFirstWhich() const
More information about the Libreoffice-commits
mailing list