ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#164843 - sc: fix ScRange hash so SubTotal re-listening stays fast

Browse Source 
Proper fix for the slowness originally reported in tdf#164843:
on a sheet with many SubTotal formulas over a relative named
range, deleting a named range hangs for ~2 min 50 sec (on a
debug build).

tdf#164843 was first "fixed" in b89047a0f0
by trimming the SubTotal double references to the data area at
compile time. That mutated the stored reference - the formula was
rewritten and rows added later were dropped - which got reported
as tdf#168013 and fixed by moving the trim to evaluation time in
the preceding commit 7ade1318a646c99926b5685d40e1e4174454bf4d
("sc: don't mutate SubTotal reference when trimming"). Removing
the compile-time trim, however, brought the original tdf#164843
slowness back: the trim had only been masking the real cause.

That cause is in ScRange::hashArea() / hashStartColumn(): each
corner was packed into a fixed high bit-field and the fields
XORed together, so only the end row reached the low-order bits
that determine the bucket index. Ranges that share their end
corner but differ only in the start corner - exactly what a
relative named range yields (A2000:A<MAX>, A2001:A<MAX>, ...)
- all hashed into a single bucket. ScBroadcastAreaSlot's
unordered_set then degraded to O(n) per insert, so re-listening
every formula after the name-range change was O(n^2): ~2 min
50 s, with the broadcast slot collapsed into one non-empty
bucket holding ~1000 areas.

Combine the corners with o3tl::hash_combine so every one influences
the low-order bits. The bucket distribution goes from 1 bucket of
1000 to 1000 buckets of 1, and the re-listen drops from ~2 min
50 sec to 2-3 sec. hashStartColumn() (used by the lookup and range
caches) had the identical defect and gets the same fix.

follow-up of: 7ade1318a646c99926b5685d40e1e4174454bf4d

Change-Id: I293f7e108cd2c788b159451061b547a11ce748bf
Signed-off-by: Balazs Varga <balazs.varga@collabora.com>
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/3654
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
(cherry picked from commit c4509310d0fcafe20397659ee6b6911f21d30bec)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/206167
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
This commit is contained in:
Balazs Varga
2026-06-05 21:53:18 +02:00
committed by Xisco Fauli
parent 2310486f88
commit b7fe1ca68a
2 changed files with 51 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sc/inc/address.hxx
+26 -36
View File
@@ -19,6 +19,7 @@
#pragma once
#include <o3tl/hash_combine.hxx>
#include <rtl/ustrbuf.hxx>
#include <rtl/strbuf.hxx>
@@ -728,46 +729,35 @@ inline bool ScRange::Intersects( const ScRange& rRange ) const
inline size_t ScRange::hashArea() const
{
#if SAL_TYPES_SIZEOFPOINTER == 8
// 12 bits for the columns and 20 bits for the rows
return
(static_cast<size_t>(aStart.Row()) << 44) ^
(static_cast<size_t>(aStart.Col()) << 32) ^
(static_cast<size_t>(aEnd.Col()) << 20) ^
static_cast<size_t>(aEnd.Row());
#else
// Assume that there are not that many ranges with identical corners so we
// won't have too many collisions. Also assume that more lower row and
// column numbers are used so that there are not too many conflicts with
// the columns hashed into the values, and that start row and column
// usually don't exceed certain values. High bits are not masked off and
// may overlap with lower bits of other values, e.g. if start column is
// greater than assumed.
return
(static_cast<size_t>(aStart.Row()) << 26) ^ // start row <= 2^6
(static_cast<size_t>(aStart.Col()) << 21) ^ // start column <= 2^5
(static_cast<size_t>(aEnd.Col()) << 15) ^ // end column <= 2^6
static_cast<size_t>(aEnd.Row()); // end row <= 2^15
#endif
// Mix all four corners into the hash so each one affects its low-order bits,
// which are what determine the bucket index in a hash table. The previous
// shift+xor parked the start column/row in high bits, so ranges that share
// the same end corner but differ only in the start corner - e.g. a relative
// named range resolving to A1:X, A2:X, A3:X, ... or repeated large LOOKUP()
// areas - all hashed to a single bucket, turning unordered_set insert/find
// into O(n). tdf#164843
size_t h = 0;
o3tl::hash_combine(h, aStart.Row());
o3tl::hash_combine(h, aStart.Col());
o3tl::hash_combine(h, aEnd.Col());
o3tl::hash_combine(h, aEnd.Row());
return h;
}
inline size_t ScRange::hashStartColumn() const
{
#if SAL_TYPES_SIZEOFPOINTER == 8
// 20 bits for the rows
return
(static_cast<size_t>(aStart.Col()) << 40) ^
(static_cast<size_t>(aStart.Row()) << 20) ^
static_cast<size_t>(aEnd.Row());
#else
// Assume that for the start row more lower row numbers are used so that
// there are not too many conflicts with the column hashed into the higher
// values.
return
(static_cast<size_t>(aStart.Col()) << 24) ^ // start column <= 2^8
(static_cast<size_t>(aStart.Row()) << 16) ^ // start row <= 2^8
static_cast<size_t>(aEnd.Row());
#endif
// As in hashArea(): mix the fields so each one affects the low-order bits
// that determine the bucket index. The previous shift+xor parked the start
// column/row in high bits, so ranges that share the same end row but differ
// only in the start corner hashed to a single bucket and made the lookup and
// range caches O(n) per probe. Keyed on the start column and the row span
// (end column intentionally omitted - lookups are on the first column).
// tdf#164843
size_t h = 0;
o3tl::hash_combine(h, aStart.Col());
o3tl::hash_combine(h, aStart.Row());
o3tl::hash_combine(h, aEnd.Row());
return h;
}
[[nodiscard]] inline bool ValidRange( const ScRange& rRange, SCCOL nMaxCol, SCROW nMaxRow )
sc/qa/unit/ucalc_range.cxx
+25
View File
@@ -18,6 +18,8 @@
#include <rangeutl.hxx>
#include <refupdatecontext.hxx>
#include <unordered_set>
class ScRangeTest : public ScUcalcTestBase
{
};
@@ -71,6 +73,29 @@ CPPUNIT_TEST_FIXTURE(ScRangeTest, testTdf147451)
CPPUNIT_ASSERT_MESSAGE("Should fail to parse.", !(nRes & ScRefFlags::VALID));
}
CPPUNIT_TEST_FIXTURE(ScRangeTest, testHashDistribution)
{
// tdf#164843: ranges sharing an end corner but differing only in the start
// corner must spread across hash buckets. The broken hash put all of them in
// a single bucket (distinct buckets == 1); a good hash spreads ~1000 ranges
// across most of the 1024 buckets.
constexpr size_t nCount = 1000;
constexpr size_t nBuckets = 1024;
std::unordered_set<size_t> aAreaBuckets;
std::unordered_set<size_t> aStartColBuckets;
for (size_t i = 0; i < nCount; ++i)
{
// varying start row, shared end row (near MAXROW)
const ScRange aRange(28, static_cast<SCROW>(2000 + i), 0, 28, 1048575, 0);
aAreaBuckets.insert(aRange.hashArea() % nBuckets);
aStartColBuckets.insert(aRange.hashStartColumn() % nBuckets);
}
CPPUNIT_ASSERT_GREATER(nCount / 2, aAreaBuckets.size());
CPPUNIT_ASSERT_GREATER(nCount / 2, aStartColBuckets.size());
}
class ScRangeUpdaterTest : public CppUnit::TestFixture
{
public: