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>

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,

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;}

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); }

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

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;

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

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;

svl/source/items/poolio.cxx

@@ -33,6 +33,7 @@
 void SfxPoolItemArray_Impl::clear()
 {
     maPoolItemSet.clear();
+    maSortablePoolItems.clear();
 }
 
 sal_uInt16 SfxItemPool::GetFirstWhich() const