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@collabora.co.uk>
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index c1c017c..c3800cc 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -152,6 +152,11 @@
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 9e4a556..e03d1a2 100644
--- a/sc/inc/attrib.hxx
+++ b/sc/inc/attrib.hxx
@@ -269,6 +269,8 @@
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 8004a57..52fd853 100644
--- a/sc/inc/patattr.hxx
+++ b/sc/inc/patattr.hxx
@@ -65,6 +65,8 @@
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 43cb67ec..37c9c51 100644
--- a/sc/source/core/data/attrib.cxx
+++ b/sc/source/core/data/attrib.cxx
@@ -682,7 +682,21 @@
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 4c3c937..bff76e3 100644
--- a/sc/source/core/data/patattr.cxx
+++ b/sc/source/core/data/patattr.cxx
@@ -111,7 +111,30 @@
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 @@
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 @@
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 8eef10c..8054063 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 @@
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 @@
{
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 @@
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 b407889..67f141d 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -628,12 +628,25 @@
}
// 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 @@
// 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 478fb82..a909450 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
