[Libreoffice-commits] core.git: include/svl include/svx svl/source svx/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Tue May 7 08:54:53 UTC 2019
include/svl/custritm.hxx | 4 -
include/svl/itempool.hxx | 6 ++
include/svx/xbtmpit.hxx | 2
include/svx/xcolit.hxx | 2
include/svx/xflgrit.hxx | 2
include/svx/xflhtit.hxx | 2
include/svx/xlndsit.hxx | 2
include/svx/xlnedit.hxx | 2
include/svx/xlnstit.hxx | 2
svl/source/inc/poolio.hxx | 92 +++++++++++++++++++++++++++-----
svl/source/items/itempool.cxx | 15 +++++
svx/source/unodraw/UnoNameItemTable.cxx | 40 +++++--------
12 files changed, 120 insertions(+), 51 deletions(-)
New commits:
commit f1ed27eed68228edbab5eb63951a602263e4c3a7
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Sat May 4 14:38:07 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue May 7 10:54:20 2019 +0200
tdf#63640 FILEOPEN/FILESAVE: particular .odt loads/saves very slow, part2
Use the existing sorting functionality in SfxItemPool and extend it to
search for NameOrIndex item in SvxUnoNameItemTable
This is a little tricky in that we are defining only a partial ordering
over the CntUnencodedStringItem (and their subclasses) items.
Partial because I can only use the part of
the item that is not randomly mutated by various code, which is why
the other fields in the subclasses are mostly out of bounds.
I had to exclude FillBitmapItem because it triggers a unit test
failure and I cannot figure out why that specific item does
not play nice with this optimisation.
After this optimisation, the load time goes from
3.6s to 2s on my machine.
Change-Id: I52d58c68db2536b69a7b0a9611a2b4c703bc4928
Reviewed-on: https://gerrit.libreoffice.org/71461
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/include/svl/custritm.hxx b/include/svl/custritm.hxx
index 3899df7b2fbc..5677c990569e 100644
--- a/include/svl/custritm.hxx
+++ b/include/svl/custritm.hxx
@@ -23,6 +23,7 @@
#include <svl/svldllapi.h>
#include <tools/debug.hxx>
#include <svl/poolitem.hxx>
+#include <cassert>
class SVL_DLLPUBLIC CntUnencodedStringItem: public SfxPoolItem
{
@@ -60,8 +61,7 @@ public:
inline void CntUnencodedStringItem::SetValue(const OUString & rTheValue)
{
- DBG_ASSERT(GetRefCount() == 0,
- "CntUnencodedStringItem::SetValue(): Pooled item");
+ assert(GetRefCount() == 0 && "cannot modify name of pooled item");
m_aValue = rTheValue;
}
diff --git a/include/svl/itempool.hxx b/include/svl/itempool.hxx
index 7f74cb85c11d..3a2ff084d723 100644
--- a/include/svl/itempool.hxx
+++ b/include/svl/itempool.hxx
@@ -171,6 +171,12 @@ public:
sal_uInt32 GetItemCount2(sal_uInt16 nWhich) const;
Item2Range GetItemSurrogates(sal_uInt16 nWhich) const;
+ /*
+ This is only valid for SfxPoolItem that override IsSortable and operator<.
+ Returns a range of items defined by using operator<.
+ @param rNeedle must be the same type or a supertype of the pool items for nWhich.
+ */
+ std::vector<const SfxPoolItem*> FindItemSurrogate(sal_uInt16 nWhich, SfxPoolItem const & rNeedle) const;
sal_uInt16 GetFirstWhich() const;
sal_uInt16 GetLastWhich() const;
diff --git a/include/svx/xbtmpit.hxx b/include/svx/xbtmpit.hxx
index dedff05369ec..8987aaf107a4 100644
--- a/include/svx/xbtmpit.hxx
+++ b/include/svx/xbtmpit.hxx
@@ -42,7 +42,7 @@ public:
XFillBitmapItem( const XFillBitmapItem& rItem );
virtual bool operator==( const SfxPoolItem& rItem ) const override;
- // NameOrIndex is sortable, but we are not
+ // no idea why, but this item does not play nice with the sorting optimisation, get failures in sd_import_tests
virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone( SfxItemPool* pPool = nullptr ) const override;
diff --git a/include/svx/xcolit.hxx b/include/svx/xcolit.hxx
index 63d5e475ec31..df3723542516 100644
--- a/include/svx/xcolit.hxx
+++ b/include/svx/xcolit.hxx
@@ -47,8 +47,6 @@ public:
XColorItem(const XColorItem& rItem);
virtual bool operator==(const SfxPoolItem& rItem) const override;
- // NameOrIndex is sortable, but we are not
- virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone(SfxItemPool* pPool = nullptr) const override;
const Color& GetColorValue() const;
diff --git a/include/svx/xflgrit.hxx b/include/svx/xflgrit.hxx
index 8e931cb40ef0..159c6862dc79 100644
--- a/include/svx/xflgrit.hxx
+++ b/include/svx/xflgrit.hxx
@@ -42,8 +42,6 @@ public:
XFillGradientItem(const XFillGradientItem& rItem);
virtual bool operator==(const SfxPoolItem& rItem) const override;
- // NameOrIndex is sortable, but we are not
- virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone(SfxItemPool* pPool = nullptr) const override;
virtual bool QueryValue( css::uno::Any& rVal, sal_uInt8 nMemberId = 0 ) const override;
diff --git a/include/svx/xflhtit.hxx b/include/svx/xflhtit.hxx
index 1f85088ad7cb..a81e7232abfa 100644
--- a/include/svx/xflhtit.hxx
+++ b/include/svx/xflhtit.hxx
@@ -41,8 +41,6 @@ public:
XFillHatchItem(const XFillHatchItem& rItem);
virtual bool operator==(const SfxPoolItem& rItem) const override;
- // NameOrIndex is sortable, but we are not
- virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone(SfxItemPool* pPool = nullptr) const override;
virtual bool QueryValue( css::uno::Any& rVal, sal_uInt8 nMemberId = 0 ) const override;
diff --git a/include/svx/xlndsit.hxx b/include/svx/xlndsit.hxx
index 8afc71545301..a19e50608d15 100644
--- a/include/svx/xlndsit.hxx
+++ b/include/svx/xlndsit.hxx
@@ -42,8 +42,6 @@ public:
XLineDashItem(const XLineDashItem& rItem);
virtual bool operator==(const SfxPoolItem& rItem) const override;
- // NameOrIndex is sortable, but we are not
- virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone(SfxItemPool* pPool = nullptr) const override;
virtual bool QueryValue( css::uno::Any& rVal, sal_uInt8 nMemberId = 0 ) const override;
diff --git a/include/svx/xlnedit.hxx b/include/svx/xlnedit.hxx
index 2896138a7e87..b24d4825946f 100644
--- a/include/svx/xlnedit.hxx
+++ b/include/svx/xlnedit.hxx
@@ -41,8 +41,6 @@ public:
XLineEndItem(const XLineEndItem& rItem);
virtual bool operator==(const SfxPoolItem& rItem) const override;
- // NameOrIndex is sortable, but we are not
- virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone(SfxItemPool* pPool = nullptr) const override;
virtual bool QueryValue( css::uno::Any& rVal, sal_uInt8 nMemberId = 0 ) const override;
diff --git a/include/svx/xlnstit.hxx b/include/svx/xlnstit.hxx
index c37cfc3bdd6d..3e9c402e8bb9 100644
--- a/include/svx/xlnstit.hxx
+++ b/include/svx/xlnstit.hxx
@@ -41,8 +41,6 @@ public:
XLineStartItem(const XLineStartItem& rItem);
virtual bool operator==(const SfxPoolItem& rItem) const override;
- // NameOrIndex is sortable, but we are not
- virtual bool IsSortable() const override { return false; }
virtual SfxPoolItem* Clone(SfxItemPool* pPool = nullptr) const override;
virtual bool QueryValue( css::uno::Any& rVal, sal_uInt8 nMemberId = 0 ) const override;
diff --git a/svl/source/inc/poolio.hxx b/svl/source/inc/poolio.hxx
index cc2039b97a66..2939d50aaf2f 100644
--- a/svl/source/inc/poolio.hxx
+++ b/svl/source/inc/poolio.hxx
@@ -33,13 +33,10 @@ class SfxItemPoolUser;
static const sal_uInt32 SFX_ITEMS_DEFAULT = 0xfffffffe;
-struct CompareSortablePoolItems
+static bool CompareSortablePoolItems(SfxPoolItem const* lhs, SfxPoolItem const* rhs)
{
- bool operator()(SfxPoolItem const* lhs, SfxPoolItem const* rhs) const
- {
- return (*lhs) < (*rhs);
- }
-};
+ 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
@@ -50,7 +47,11 @@ struct SfxPoolItemArray_Impl
{
private:
o3tl::sorted_vector<SfxPoolItem*> maPoolItemSet;
- o3tl::sorted_vector<const SfxPoolItem*, CompareSortablePoolItems> maSortablePoolItems;
+ // In some cases, e.g. subclasses of NameOrIndex, the parent class (NameOrIndex) is sortable,
+ // but the subclasses do not define an operator<, which means that we don't get an ordering
+ // strong enough to enforce uniqueness purely with operator<, which means we need to do
+ // a partial scan with operator==
+ std::vector<SfxPoolItem*> maSortablePoolItems;
public:
o3tl::sorted_vector<SfxPoolItem*>::const_iterator begin() const { return maPoolItemSet.begin(); }
o3tl::sorted_vector<SfxPoolItem*>::const_iterator end() const { return maPoolItemSet.end(); }
@@ -59,22 +60,87 @@ public:
void clear();
size_t size() const {return maPoolItemSet.size();}
bool empty() const {return maPoolItemSet.empty();}
+ o3tl::sorted_vector<SfxPoolItem*>::const_iterator find(SfxPoolItem* pItem) const { return maPoolItemSet.find(pItem); }
void insert(SfxPoolItem* pItem)
{
maPoolItemSet.insert(pItem);
if (pItem->IsSortable())
- maSortablePoolItems.insert(pItem);
+ {
+ // bail early if someone modified one of these things underneath me
+ assert( std::is_sorted_until(maSortablePoolItems.begin(), maSortablePoolItems.end(), CompareSortablePoolItems) == maSortablePoolItems.end());
+
+ auto it = std::lower_bound(maSortablePoolItems.begin(), maSortablePoolItems.end(), pItem, CompareSortablePoolItems);
+ maSortablePoolItems.insert(maSortablePoolItems.begin() + (it - maSortablePoolItems.begin()), pItem);
+ }
}
- o3tl::sorted_vector<SfxPoolItem*>::const_iterator find(SfxPoolItem* pItem) const { return maPoolItemSet.find(pItem); }
- const SfxPoolItem* findByLessThan(const SfxPoolItem* pItem) const
+ const SfxPoolItem* findByLessThan(const SfxPoolItem* pNeedle) const
+ {
+ // bail early if someone modified one of these things underneath me
+ assert( std::is_sorted_until(maSortablePoolItems.begin(), maSortablePoolItems.end(), CompareSortablePoolItems) == maSortablePoolItems.end());
+ assert( maPoolItemSet.empty() || maPoolItemSet.front()->IsSortable() );
+
+ auto it = std::lower_bound(maSortablePoolItems.begin(), maSortablePoolItems.end(), pNeedle, CompareSortablePoolItems);
+ for (;;)
+ {
+ if (it == maSortablePoolItems.end())
+ return nullptr;
+ if (**it < *pNeedle)
+ return nullptr;
+ if (*pNeedle == **it)
+ return *it;
+ ++it;
+ }
+ }
+ std::vector<const SfxPoolItem*> findSurrogateRange(const SfxPoolItem* pNeedle) const
{
- auto it = maSortablePoolItems.find(pItem);
- return it == maSortablePoolItems.end() ? nullptr : *it;
+ std::vector<const SfxPoolItem*> rv;
+ if (!maSortablePoolItems.empty())
+ {
+ // bail early if someone modified one of these things underneath me
+ assert( std::is_sorted_until(maSortablePoolItems.begin(), maSortablePoolItems.end(), CompareSortablePoolItems) == maSortablePoolItems.end());
+
+ auto range = std::equal_range(maSortablePoolItems.begin(), maSortablePoolItems.end(), pNeedle, CompareSortablePoolItems);
+ rv.reserve(std::distance(range.first, range.second));
+ for (auto it = range.first; it != range.second; ++it)
+ rv.push_back(*it);
+ }
+ else
+ {
+ for (const SfxPoolItem* p : maPoolItemSet)
+ if (*pNeedle == *p)
+ rv.push_back(p);
+ }
+ return rv;
}
void erase(o3tl::sorted_vector<SfxPoolItem*>::const_iterator it)
{
+ auto pNeedle = *it;
if ((*it)->IsSortable())
- maSortablePoolItems.erase(*it);
+ {
+ // bail early if someone modified one of these things underneath me
+ assert( std::is_sorted_until(maSortablePoolItems.begin(), maSortablePoolItems.end(), CompareSortablePoolItems) == maSortablePoolItems.end());
+
+ auto sortIt = std::lower_bound(maSortablePoolItems.begin(), maSortablePoolItems.end(), pNeedle, CompareSortablePoolItems);
+ for (;;)
+ {
+ if (sortIt == maSortablePoolItems.end())
+ {
+ assert(false && "did not find item?");
+ break;
+ }
+ if (**sortIt < *pNeedle)
+ {
+ assert(false && "did not find item?");
+ break;
+ }
+ if (**sortIt == *pNeedle)
+ {
+ maSortablePoolItems.erase(sortIt);
+ break;
+ }
+ ++sortIt;
+ }
+ }
return maPoolItemSet.erase(it);
}
};
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index 7c6f746ba319..70809ac65ad4 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -843,6 +843,21 @@ SfxItemPool::Item2Range SfxItemPool::GetItemSurrogates(sal_uInt16 nWhich) const
return { rItemArr.begin(), rItemArr.end() };
}
+/* This is only valid for SfxPoolItem that override IsSortable and operator< */
+std::vector<const SfxPoolItem*> SfxItemPool::FindItemSurrogate(sal_uInt16 nWhich, SfxPoolItem const & rSample) const
+{
+ if ( !IsInRange(nWhich) )
+ {
+ if ( pImpl->mpSecondary )
+ return pImpl->mpSecondary->FindItemSurrogate( nWhich, rSample );
+ assert(false && "unknown WhichId - cannot resolve surrogate");
+ return std::vector<const SfxPoolItem*>();
+ }
+
+ SfxPoolItemArray_Impl& rItemArr = pImpl->maPoolItemArrays[GetIndex_Impl(nWhich)];
+ return rItemArr.findSurrogateRange(&rSample);
+}
+
sal_uInt32 SfxItemPool::GetItemCount2(sal_uInt16 nWhich) const
{
if ( !IsInRange(nWhich) )
diff --git a/svx/source/unodraw/UnoNameItemTable.cxx b/svx/source/unodraw/UnoNameItemTable.cxx
index 5a27573e62f2..56fe86d489ca 100644
--- a/svx/source/unodraw/UnoNameItemTable.cxx
+++ b/svx/source/unodraw/UnoNameItemTable.cxx
@@ -159,16 +159,15 @@ void SAL_CALL SvxUnoNameItemTable::replaceByName( const OUString& aApiName, cons
bool bFound = false;
if (mpModelPool)
- for (const SfxPoolItem* pItem : mpModelPool->GetItemSurrogates(mnWhich))
- {
- NameOrIndex *pNameOrIndex = const_cast<NameOrIndex*>(static_cast<const NameOrIndex*>(pItem));
- if (pNameOrIndex && aName == pNameOrIndex->GetName())
+ {
+ NameOrIndex aSample(mnWhich, aName);
+ for (const SfxPoolItem* pNameOrIndex : mpModelPool->FindItemSurrogate(mnWhich, aSample))
+ if (isValid(static_cast<const NameOrIndex*>(pNameOrIndex)))
{
- pNameOrIndex->PutValue( aElement, mnMemberId );
+ const_cast<SfxPoolItem*>(pNameOrIndex)->PutValue( aElement, mnMemberId );
bFound = true;
- break;
}
- }
+ }
if( !bFound )
throw container::NoSuchElementException();
@@ -187,20 +186,16 @@ uno::Any SAL_CALL SvxUnoNameItemTable::getByName( const OUString& aApiName )
OUString aName = SvxUnogetInternalNameForItem(mnWhich, aApiName);
- uno::Any aAny;
-
if (mpModelPool && !aName.isEmpty())
{
- for (const SfxPoolItem* pItem : mpModelPool->GetItemSurrogates(mnWhich))
- {
- const NameOrIndex *pNameOrIndex = static_cast<const NameOrIndex*>(pItem);
-
- if (isValid(pNameOrIndex) && aName == pNameOrIndex->GetName())
+ NameOrIndex aSample(mnWhich, aName);
+ for (const SfxPoolItem* pFindItem : mpModelPool->FindItemSurrogate(mnWhich, aSample))
+ if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
{
- pNameOrIndex->QueryValue( aAny, mnMemberId );
+ uno::Any aAny;
+ pFindItem->QueryValue( aAny, mnMemberId );
return aAny;
}
- }
}
throw container::NoSuchElementException();
@@ -237,14 +232,13 @@ sal_Bool SAL_CALL SvxUnoNameItemTable::hasByName( const OUString& aApiName )
if (aName.isEmpty())
return false;
- if (mpModelPool)
- for (const SfxPoolItem* pItem : mpModelPool->GetItemSurrogates(mnWhich))
- {
- const NameOrIndex *pNameOrIndex = static_cast<const NameOrIndex*>(pItem);
- if (isValid(pNameOrIndex) && aName == pNameOrIndex->GetName())
- return true;
- }
+ if (!mpModelPool)
+ return false;
+ NameOrIndex aSample(mnWhich, aName);
+ for (const SfxPoolItem* pFindItem : mpModelPool->FindItemSurrogate(mnWhich, aSample))
+ if (isValid(static_cast<const NameOrIndex*>(pFindItem)))
+ return true;
return false;
}
More information about the Libreoffice-commits
mailing list