summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--sc/inc/patattr.hxx6
-rw-r--r--sc/source/core/data/attarray.cxx13
-rw-r--r--sc/source/core/data/global.cxx12
-rw-r--r--sc/source/core/data/patattr.cxx128
-rw-r--r--svl/source/items/itemset.cxx108
5 files changed, 152 insertions, 115 deletions
diff --git a/sc/inc/patattr.hxx b/sc/inc/patattr.hxx
index b5deb19826d9..d6536551f965 100644
--- a/sc/inc/patattr.hxx
+++ b/sc/inc/patattr.hxx
@@ -53,7 +53,6 @@ enum class ScAutoFontColorMode
class SC_DLLPUBLIC ScPatternAttr final : public SfxSetItem
{
std::optional<OUString> pName;
- mutable std::optional<sal_uInt32> mxHashCode;
mutable std::optional<bool> mxVisible;
ScStyleSheet* pStyle;
sal_uInt64 mnPAKey;
@@ -181,16 +180,13 @@ public:
void SetPAKey(sal_uInt64 nKey);
sal_uInt64 GetPAKey() const;
- static std::optional<bool> FastEqualPatternSets( const SfxItemSet& rSet1, const SfxItemSet& rSet2 );
-
// TODO: tdf#135215: This is a band-aid to detect changes and invalidate the hash,
// a proper way would be probably to override SfxItemSet::Changed(), but 6cb400f41df0dd10
// hardcoded SfxSetItem to contain SfxItemSet.
- SfxItemSet& GetItemSet() { mxHashCode.reset(); mxVisible.reset(); return SfxSetItem::GetItemSet(); }
+ SfxItemSet& GetItemSet() { mxVisible.reset(); return SfxSetItem::GetItemSet(); }
using SfxSetItem::GetItemSet;
private:
- void CalcHashCode() const;
bool CalcVisible() const;
};
diff --git a/sc/source/core/data/attarray.cxx b/sc/source/core/data/attarray.cxx
index 3e56226f3704..c2f3b0b75195 100644
--- a/sc/source/core/data/attarray.cxx
+++ b/sc/source/core/data/attarray.cxx
@@ -294,10 +294,13 @@ void ScAttrArray::AddCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 nInd
{
const ScPatternAttr* pPattern = GetPattern(nTempStartRow);
+ // changed to create pNewPattern only if needed, else use already
+ // existing pPattern. This shows by example how to avoid that special
+ // handling of ATTR_PATTERN/ScPatternAttr in SC and massive
+ // incarnations/desctructions of that Item (which contains an ItemSet)
std::unique_ptr<ScPatternAttr> pNewPattern;
if(pPattern)
{
- pNewPattern.reset( new ScPatternAttr(*pPattern) );
SCROW nPatternStartRow;
SCROW nPatternEndRow;
GetPatternRange( nPatternStartRow, nPatternEndRow, nTempStartRow );
@@ -313,12 +316,14 @@ void ScAttrArray::AddCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 nInd
aNewCondFormatData = rCondFormatData;
aNewCondFormatData.insert(nIndex);
ScCondFormatItem aItem( std::move(aNewCondFormatData) );
+ pNewPattern.reset( new ScPatternAttr(*pPattern) );
pNewPattern->GetItemSet().Put( aItem );
}
}
else
{
ScCondFormatItem aItem(nIndex);
+ pNewPattern.reset( new ScPatternAttr(*pPattern) );
pNewPattern->GetItemSet().Put( aItem );
}
}
@@ -330,7 +335,11 @@ void ScAttrArray::AddCondFormat( SCROW nStartRow, SCROW nEndRow, sal_uInt32 nInd
nTempEndRow = nEndRow;
}
- SetPatternArea( nTempStartRow, nTempEndRow, std::move(pNewPattern), true );
+ if (pNewPattern)
+ SetPatternArea( nTempStartRow, nTempEndRow, std::move(pNewPattern), true );
+ else
+ SetPatternArea( nTempStartRow, nTempEndRow, pPattern, true );
+
nTempStartRow = nTempEndRow + 1;
}
while(nTempEndRow < nEndRow);
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index c4737d1e0bce..29a616c6b6e8 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -187,8 +187,16 @@ bool ScGlobal::CheckWidthInvalidate( bool& bNumFormatChanged,
const SfxItemSet& rNewAttrs,
const SfxItemSet& rOldAttrs )
{
- std::optional<bool> equal = ScPatternAttr::FastEqualPatternSets( rNewAttrs, rOldAttrs );
- if( equal.has_value() && *equal )
+ // Here ScPatternAttr::FastEqualPatternSets was used before. This implies that
+ // the two given SfxItemSet are internal ones from ScPatternAttr, but there is
+ // no guarantee here for that. Also that former method contained the comment
+ // "Actually test_tdf133629 from UITest_calc_tests9 somehow manages to have
+ // a different range (and I don't understand enough why), so better be safe and compare fully."
+ // which may be based on this usage. I check for that already in
+ // ScPatternAttr::operator==, seems not to be triggered there.
+ // All in all: Better use SfxItemSet::operator== here, and not one specialized
+ // on the SfxItemSets of ScPatternAttr
+ if (rNewAttrs == rOldAttrs)
{
bNumFormatChanged = false;
return false;
diff --git a/sc/source/core/data/patattr.cxx b/sc/source/core/data/patattr.cxx
index d7d460abdbb5..7fe65bc82ba9 100644
--- a/sc/source/core/data/patattr.cxx
+++ b/sc/source/core/data/patattr.cxx
@@ -68,6 +68,8 @@
#include <comphelper/lok.hxx>
#include <tabvwsh.hxx>
+const WhichRangesContainer aScPatternAttrSchema(svl::Items<ATTR_PATTERN_START, ATTR_PATTERN_END>);
+
ScPatternAttr::ScPatternAttr( SfxItemSet&& pItemSet, const OUString& rStyleName )
: SfxSetItem ( ATTR_PATTERN, std::move(pItemSet) ),
pName ( rStyleName ),
@@ -75,6 +77,12 @@ ScPatternAttr::ScPatternAttr( SfxItemSet&& pItemSet, const OUString& rStyleName
mnPAKey(0)
{
setNewItemCallback();
+
+ // We need to ensure that ScPatternAttr is using the correct WhichRange,
+ // see comments in commit message. This does transfers the items with
+ // minimized overhead, too
+ if (GetItemSet().GetRanges() != aScPatternAttrSchema)
+ GetItemSet().SetRanges(aScPatternAttrSchema);
}
ScPatternAttr::ScPatternAttr( SfxItemSet&& pItemSet )
@@ -83,6 +91,12 @@ ScPatternAttr::ScPatternAttr( SfxItemSet&& pItemSet )
mnPAKey(0)
{
setNewItemCallback();
+
+ // We need to ensure that ScPatternAttr is using the correct WhichRange,
+ // see comments in commit message. This does transfers the items with
+ // minimized overhead, too
+ if (GetItemSet().GetRanges() != aScPatternAttrSchema)
+ GetItemSet().SetRanges(aScPatternAttrSchema);
}
ScPatternAttr::ScPatternAttr( SfxItemPool* pItemPool )
@@ -125,49 +139,63 @@ static bool StrCmp( const OUString* pStr1, const OUString* pStr2 )
constexpr size_t compareSize = ATTR_PATTERN_END - ATTR_PATTERN_START + 1;
-std::optional<bool> ScPatternAttr::FastEqualPatternSets( const SfxItemSet& rSet1, const SfxItemSet& rSet2 )
+bool ScPatternAttr::operator==( const SfxPoolItem& rCmp ) const
{
- // #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 { false };
-
- // Actually test_tdf133629 from UITest_calc_tests9 somehow manages to have
- // a different range (and I don't understand enough why), so better be safe and compare fully.
- if( rSet1.TotalCount() != compareSize || rSet2.TotalCount() != compareSize )
- return std::nullopt;
+ // check if same incarnation
+ if (this == &rCmp)
+ return true;
- SfxPoolItem const ** pItems1 = rSet1.GetItems_Impl(); // inline method of SfxItemSet
- SfxPoolItem const ** pItems2 = rSet2.GetItems_Impl();
+ // check SfxPoolItem base class
+ if (!SfxPoolItem::operator==(rCmp) )
+ return false;
- return { memcmp( pItems1, pItems2, compareSize * sizeof(pItems1[0]) ) == 0 };
-}
+ // check everything except the SfxItemSet from base class SfxSetItem
+ const ScPatternAttr& rOther(static_cast<const ScPatternAttr&>(rCmp));
+ if (!StrCmp(GetStyleName(), rOther.GetStyleName()))
+ return false;
-static bool EqualPatternSets( const SfxItemSet& rSet1, const SfxItemSet& rSet2 )
-{
- std::optional<bool> equal = ScPatternAttr::FastEqualPatternSets( rSet1, rSet2 );
- if(equal.has_value())
- return *equal;
- return rSet1 == rSet2;
-}
+ // here we need to compare the SfxItemSet. We *know* that these are
+ // all simple (one range, same range)
+ const SfxItemSet& rSet1(GetItemSet());
+ const SfxItemSet& rSet2(rOther.GetItemSet());
-bool ScPatternAttr::operator==( const SfxPoolItem& rCmp ) const
-{
- // #i62090# Use quick comparison between ScPatternAttr's ItemSets
+ // the former method 'FastEqualPatternSets' mentioned:
+ // "Actually test_tdf133629 from UITest_calc_tests9 somehow manages to have
+ // a different range (and I don't understand enough why), so better be safe and compare fully."
+ // in that case the hash code above would already fail, too
+ if (rSet1.TotalCount() != compareSize || rSet2.TotalCount() != compareSize)
+ {
+ // assert this for now, should not happen. If it does, look for it and evtl.
+ // enable SfxItemSet::operator== below
+ assert(false);
+ return rSet1 == rSet2;
+ }
- if (!SfxPoolItem::operator==(rCmp) )
+ // check pools, do not accept different pools
+ if (rSet1.GetPool() != rSet2.GetPool())
return false;
- if (!mxHashCode)
- CalcHashCode();
- auto const & rOther = static_cast<const ScPatternAttr&>(rCmp);
- if (!rOther.mxHashCode)
- rOther.CalcHashCode();
- if (*mxHashCode != *rOther.mxHashCode)
+
+ // check count of set items, has to be equal
+ if (rSet1.Count() != rSet2.Count())
return false;
- return EqualPatternSets( GetItemSet(), rOther.GetItemSet() ) &&
- StrCmp( GetStyleName(), rOther.GetStyleName() );
+
+ // compare each item separately
+ const SfxPoolItem **ppItem1(rSet1.GetItems_Impl());
+ const SfxPoolItem **ppItem2(rSet2.GetItems_Impl());
+
+ // are all pointers the same?
+ if (0 == memcmp(ppItem1, ppItem2, compareSize * sizeof(ppItem1[0])))
+ return true;
+
+ for (sal_uInt16 nPos(0); nPos < compareSize; nPos++)
+ {
+ if (!SfxPoolItem::areSame(*ppItem1, *ppItem2))
+ return false;
+ ++ppItem1;
+ ++ppItem2;
+ }
+
+ return true;
}
SvxCellOrientation ScPatternAttr::GetCellOrientation( const SfxItemSet& rItemSet, const SfxItemSet* pCondSet )
@@ -971,7 +999,6 @@ void ScPatternAttr::GetFromEditItemSet( const SfxItemSet* pEditSet )
if( !pEditSet )
return;
GetFromEditItemSet( GetItemSet(), *pEditSet );
- mxHashCode.reset();
mxVisible.reset();
}
@@ -1015,7 +1042,6 @@ void ScPatternAttr::DeleteUnchanged( const ScPatternAttr* pOldAttrs )
if (SfxPoolItem::areSame( pThisItem, pOldItem ))
{
rThisSet.ClearItem( nSubWhich );
- mxHashCode.reset();
mxVisible.reset();
}
}
@@ -1025,7 +1051,6 @@ void ScPatternAttr::DeleteUnchanged( const ScPatternAttr* pOldAttrs )
if ( *pThisItem == rThisSet.GetPool()->GetDefaultItem( nSubWhich ) )
{
rThisSet.ClearItem( nSubWhich );
- mxHashCode.reset();
mxVisible.reset();
}
}
@@ -1047,7 +1072,6 @@ void ScPatternAttr::ClearItems( const sal_uInt16* pWhich )
SfxItemSet& rSet = GetItemSet();
for (sal_uInt16 i=0; pWhich[i]; i++)
rSet.ClearItem(pWhich[i]);
- mxHashCode.reset();
mxVisible.reset();
}
@@ -1313,7 +1337,6 @@ void ScPatternAttr::SetStyleSheet( ScStyleSheet* pNewStyle, bool bClearDirectFor
GetItemSet().SetParent(nullptr);
pStyle = nullptr;
}
- mxHashCode.reset();
mxVisible.reset();
}
@@ -1340,7 +1363,6 @@ void ScPatternAttr::UpdateStyleSheet(const ScDocument& rDoc)
}
else
pStyle = nullptr;
- mxHashCode.reset();
mxVisible.reset();
}
@@ -1353,7 +1375,6 @@ void ScPatternAttr::StyleToName()
pName = pStyle->GetName();
pStyle = nullptr;
GetItemSet().SetParent( nullptr );
- mxHashCode.reset();
mxVisible.reset();
}
}
@@ -1489,27 +1510,4 @@ sal_uInt64 ScPatternAttr::GetPAKey() const
return mnPAKey;
}
-void ScPatternAttr::CalcHashCode() const
-{
- auto const & rSet = GetItemSet();
- // This is an unrolled hash function so the compiler/CPU can execute it in parallel,
- // because we hit this hard when loading documents with lots of styles.
- sal_uInt32 h1 = 0;
- sal_uInt32 h2 = 0;
- sal_uInt32 h3 = 0;
- sal_uInt32 h4 = 0;
- for (auto it = rSet.GetItems_Impl(), end = rSet.GetItems_Impl() + (compareSize / 4 * 4); it != end; )
- {
- h1 = 31 * h1 + reinterpret_cast<size_t>(*it);
- ++it;
- h2 = 31 * h2 + reinterpret_cast<size_t>(*it);
- ++it;
- h3 = 31 * h3 + reinterpret_cast<size_t>(*it);
- ++it;
- h4 = 31 * h4 + reinterpret_cast<size_t>(*it);
- ++it;
- }
- mxHashCode = h1 + h2 + h3 + h4;
-}
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 8c024ae6a768..6cf4e706e17d 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -1482,60 +1482,86 @@ bool SfxItemSet::operator==(const SfxItemSet &rCmp) const
bool SfxItemSet::Equals(const SfxItemSet &rCmp, bool bComparePool) const
{
- // Values we can get quickly need to be the same
- const bool bDifferentPools = (GetPool() != rCmp.GetPool());
- if ( (bComparePool && GetParent() != rCmp.GetParent()) ||
- (bComparePool && bDifferentPools) ||
- Count() != rCmp.Count() )
+ // check if same incarnation
+ if (this == &rCmp)
+ return true;
+
+ // check parents (if requested, also bComparePool)
+ if (bComparePool && GetParent() != rCmp.GetParent())
return false;
- // If we reach here and bDifferentPools==true that means bComparePool==false.
+ // check pools (if requested)
+ if (bComparePool && GetPool() != rCmp.GetPool())
+ return false;
- // Counting Ranges takes longer; they also need to be the same, however
- const sal_uInt16 nCount1(TotalCount());
- const sal_uInt16 nCount2(rCmp.TotalCount());
- if ( nCount1 != nCount2 )
+ // check count of set items
+ if (Count() != rCmp.Count())
return false;
- // Are the Ranges themselves unequal?
- for (sal_Int32 i = 0; i < GetRanges().size(); ++i)
- {
- if (GetRanges()[i] != rCmp.GetRanges()[i])
- {
- // We must use the slow method then
- SfxWhichIter aIter( *this );
- for ( sal_uInt16 nWh = aIter.FirstWhich();
- nWh;
- nWh = aIter.NextWhich() )
- {
- // If the pointer of the shareable Items are unequal, the Items must match
- const SfxPoolItem *pItem1 = nullptr, *pItem2 = nullptr;
- if ( GetItemState_ForWhichID(SfxItemState::UNKNOWN, nWh, false, &pItem1 ) !=
- rCmp.GetItemState_ForWhichID(SfxItemState::UNKNOWN, nWh, false, &pItem2 ) ||
- !SfxPoolItem::areSame(pItem1, pItem2))
- return false;
- }
+ // both have no items, done
+ if (0 == Count())
+ return true;
+ // check if ranges are equal
+ if (GetRanges() == rCmp.GetRanges())
+ {
+ // if yes, we can simplify: are all pointers the same?
+ if (0 == memcmp( m_ppItems, rCmp.m_ppItems, TotalCount() * sizeof(m_ppItems[0]) ))
return true;
+
+ // compare each one separately
+ const SfxPoolItem **ppItem1(m_ppItems);
+ const SfxPoolItem **ppItem2(rCmp.m_ppItems);
+
+ for (sal_uInt16 nPos(0); nPos < TotalCount(); nPos++)
+ {
+ // do full SfxPoolItem compare
+ if (!SfxPoolItem::areSame(*ppItem1, *ppItem2))
+ return false;
+ ++ppItem1;
+ ++ppItem2;
}
- }
- // Are all pointers the same?
- if (0 == memcmp( m_ppItems, rCmp.m_ppItems, nCount1 * sizeof(m_ppItems[0]) ))
return true;
+ }
- // We need to compare each one separately then
- const SfxPoolItem **ppItem1 = m_ppItems;
- const SfxPoolItem **ppItem2 = rCmp.m_ppItems;
- for ( sal_uInt16 nPos = 0; nPos < nCount1; ++nPos )
+ // Not same ranges, need to compare content. Only need to check if
+ // the content of one is inside the other due to already having
+ // compared Count() above.
+ // Iterate over local SfxItemSet by using locval ranges and offset,
+ // so we can access needed data at least for one SfxItemSet more
+ // direct. For the 2nd one we need the WhichID which we have by
+ // iterating over the ranges.
+ sal_uInt16 nOffset(0);
+ sal_uInt16 nNumberToGo(Count());
+
+ for (auto const & rRange : GetRanges())
{
- // If the pointers of the shareable Items are not the same, the Items
- // must match
- if (!SfxPoolItem::areSame(*ppItem1, *ppItem2))
- return false;
+ for (sal_uInt16 nWhich(rRange.first); nWhich <= rRange.second; nWhich++, nOffset++)
+ {
+ const SfxPoolItem *pItem1(nullptr);
+ const SfxPoolItem *pItem2(nullptr);
+ const SfxItemState aStateA(GetItemState_ForOffset(nOffset, &pItem1));
+ const SfxItemState aStateB(rCmp.GetItemState_ForWhichID(SfxItemState::UNKNOWN, nWhich, false, &pItem2));
+
+ if (aStateA != aStateB)
+ return false;
+
+ // only compare items if SfxItemState::SET, else the item ptrs are not set
+ if (SfxItemState::SET == aStateA && !SfxPoolItem::areSame(pItem1, pItem2))
+ return false;
+
+ if (SfxItemState::DEFAULT != aStateA)
+ // if local item is not-nullptr we have compared one more, reduce NumberToGo
+ // NOTE: we could also use 'nullptr != *(begin() + nOffset)' here, but the
+ // entry was already checked by GetItemState_ForOffset above
+ nNumberToGo--;
- ++ppItem1;
- ++ppItem2;
+ if (0 == nNumberToGo)
+ // if we have compared Count() number of items and are still here
+ // (all were equal), we can exit early
+ return true;
+ }
}
return true;
generated by cgit (git 2.34.1) at 2024-07-11 06:02:10 +0000