From 22311e78582f88505a00803822c327a0d234b81677458d08785e96f76a3f8ef1 Mon Sep 17 00:00:00 2001 From: Danilo Spinella Date: Wed, 18 May 2022 10:39:44 +0000 Subject: [PATCH] Accepting request 977905 from LibreOffice:7.3 - Fix bsc#1197497 - LO-L3: Loading XLSX with 1M rows is ultra slow (or crashes Calc) * bsc1197497.patch OBS-URL: https://build.opensuse.org/request/show/977905 OBS-URL: https://build.opensuse.org/package/show/LibreOffice:Factory/libreoffice?expand=0&rev=1022 --- bsc1197497.patch | 286 ++++++++++++++++++++++++++++++++++++++++++++ libreoffice.changes | 6 + libreoffice.spec | 3 + 3 files changed, 295 insertions(+) create mode 100644 bsc1197497.patch diff --git a/bsc1197497.patch b/bsc1197497.patch new file mode 100644 index 0000000..d4156d7 --- /dev/null +++ b/bsc1197497.patch @@ -0,0 +1,286 @@ +From 8dfa3f0fcd057b402ebde0a4ad102956275f717e Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= +Date: Mon, 4 Apr 2022 16:30:58 +0200 +Subject: [PATCH] limit Interpret() for dirty cells only to the given row range + (bsc#1197497) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +ScColumn::InterpretDirtyCells() already takes a row range, and Interpret() +can take a range inside a formula group, so don't lose the information +in DirtyCellInterpreter, otherwise a whole large formula could be +interpreted when just a subset would be enough. + +Change-Id: I93e5a7a212976be6fd588de6f68204cd1a271348 +Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133305 +Tested-by: Jenkins +Reviewed-by: Luboš Luňák +(cherry picked from commit c315a2f7b99fc1dfbc3fc834590d22fbe41ea70f) +--- + sc/source/core/data/column3.cxx | 37 +++++++++++++++++++++++++++++++-- + 1 file changed, 35 insertions(+), 2 deletions(-) + +diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx +index 74b2e1ac7fad..7e133d565558 100644 +--- a/sc/source/core/data/column3.cxx ++++ b/sc/source/core/data/column3.cxx +@@ -102,13 +102,46 @@ void ScColumn::BroadcastRows( SCROW nStartRow, SCROW nEndRow, SfxHintId nHint ) + + namespace { + +-struct DirtyCellInterpreter ++class DirtyCellInterpreter + { ++public: + void operator() (size_t, ScFormulaCell* p) + { +- if (p->GetDirty()) ++ if(!p->GetDirty()) ++ return; ++ // Interpret() takes a range in a formula group, so group those together. ++ if( firstCell != nullptr && p->GetCellGroup() == p->GetCellGroup() ++ && p->aPos.Row() == lastPos.Row() + 1 ) ++ { ++ assert( p->aPos.Tab() == lastPos.Tab() && p->aPos.Col() == lastPos.Col()); ++ lastPos = p->aPos; // Extend range. ++ return; ++ } ++ flushPending(); ++ if( !p->GetCellGroup()) ++ { + p->Interpret(); ++ return; ++ } ++ firstCell = p; ++ lastPos = p->aPos; ++ ++ } ++ ~DirtyCellInterpreter() ++ { ++ flushPending(); ++ } ++private: ++ void flushPending() ++ { ++ if(firstCell == nullptr) ++ return; ++ SCROW firstRow = firstCell->GetCellGroup()->mpTopCell->aPos.Row(); ++ firstCell->Interpret(firstCell->aPos.Row() - firstRow, lastPos.Row() - firstRow); ++ firstCell = nullptr; + } ++ ScFormulaCell* firstCell = nullptr; ++ ScAddress lastPos; + }; + + } +-- +2.35.3 + +From 7d66afaf709154c1af9b83994f7c87afb4be254e Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= +Date: Mon, 4 Apr 2022 17:52:04 +0200 +Subject: [PATCH] try to limit cell interpreting to only visible cells when + drawing +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +If there's a document with a huge formula group, InterpretDirtyCells() +on load will only interpret the range needed for drawing. But then +scrolling just a bit could result in e.g. IsValue() call on a cell, +and that could result in unrestricted Interpret() on the whole +huge formula group, which could be slow. So explicitly interpret +just the drawn cells in the hope that it'll avoid any further +Interpret() calls. + +Change-Id: I01c9f95cf8a1cf240b798feef27d21010957030c +Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133306 +Tested-by: Jenkins +Reviewed-by: Luboš Luňák +(cherry picked from commit 64cb1d10fffccebbc825c858083f13eb717b0553) +--- + sc/inc/column.hxx | 1 + + sc/inc/document.hxx | 2 ++ + sc/inc/table.hxx | 1 + + sc/source/core/data/column3.cxx | 39 +++++++++++++++++++++++++++----- + sc/source/core/data/document.cxx | 17 ++++++++++++++ + sc/source/core/data/table1.cxx | 7 ++++++ + sc/source/ui/view/output2.cxx | 5 ++++ + 7 files changed, 66 insertions(+), 6 deletions(-) + +diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx +index 01e58fb9d055..b8e348d00e70 100644 +--- a/sc/inc/column.hxx ++++ b/sc/inc/column.hxx +@@ -651,6 +651,7 @@ public: + bool IsDrawObjectsEmptyBlock(SCROW nStartRow, SCROW nEndRow) const; + + void InterpretDirtyCells( SCROW nRow1, SCROW nRow2 ); ++ void InterpretCellsIfNeeded( SCROW nRow1, SCROW nRow2 ); + + static void JoinNewFormulaCell( const sc::CellStoreType::position_type& aPos, ScFormulaCell& rCell ); + +diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx +index 029a339f94a7..89ca4dee068a 100644 +--- a/sc/inc/document.hxx ++++ b/sc/inc/document.hxx +@@ -1318,6 +1318,8 @@ public: + void SetDirty( const ScRange&, bool bIncludeEmptyCells ); + void SetTableOpDirty( const ScRange& ); // for Interpreter TableOp + void InterpretDirtyCells( const ScRangeList& rRanges ); ++ // Interprets cells that have NeedsInterpret(), i.e. the same like calling MaybeInterpret() on them. ++ void InterpretCellsIfNeeded( const ScRangeList& rRanges ); + SC_DLLPUBLIC void CalcAll(); + SC_DLLPUBLIC void CalcAfterLoad( bool bStartListening = true ); + void CompileAll(); +diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx +index 5602d6dcacfe..27a1fcf67fd8 100644 +--- a/sc/inc/table.hxx ++++ b/sc/inc/table.hxx +@@ -1031,6 +1031,7 @@ public: + void FillMatrix( ScMatrix& rMat, SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2, svl::SharedStringPool* pPool ) const; + + void InterpretDirtyCells( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2 ); ++ void InterpretCellsIfNeeded( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2 ); + + void SetFormulaResults( SCCOL nCol, SCROW nRow, const double* pResults, size_t nLen ); + +diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx +index 7e133d565558..d3c17d1da6d7 100644 +--- a/sc/source/core/data/column3.cxx ++++ b/sc/source/core/data/column3.cxx +@@ -102,13 +102,11 @@ void ScColumn::BroadcastRows( SCROW nStartRow, SCROW nEndRow, SfxHintId nHint ) + + namespace { + +-class DirtyCellInterpreter ++class CellInterpreterBase + { +-public: +- void operator() (size_t, ScFormulaCell* p) ++protected: ++ void Interpret(ScFormulaCell* p) + { +- if(!p->GetDirty()) +- return; + // Interpret() takes a range in a formula group, so group those together. + if( firstCell != nullptr && p->GetCellGroup() == p->GetCellGroup() + && p->aPos.Row() == lastPos.Row() + 1 ) +@@ -127,7 +125,7 @@ public: + lastPos = p->aPos; + + } +- ~DirtyCellInterpreter() ++ ~CellInterpreterBase() + { + flushPending(); + } +@@ -144,6 +142,26 @@ private: + ScAddress lastPos; + }; + ++class DirtyCellInterpreter : public CellInterpreterBase ++{ ++public: ++ void operator() (size_t, ScFormulaCell* p) ++ { ++ if(p->GetDirty()) ++ Interpret(p); ++ } ++}; ++ ++class NeedsInterpretCellInterpreter : public CellInterpreterBase ++{ ++public: ++ void operator() (size_t, ScFormulaCell* p) ++ { ++ if(p->NeedsInterpret()) ++ Interpret(p); ++ } ++}; ++ + } + + void ScColumn::InterpretDirtyCells( SCROW nRow1, SCROW nRow2 ) +@@ -155,6 +173,15 @@ void ScColumn::InterpretDirtyCells( SCROW nRow1, SCROW nRow2 ) + sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aFunc); + } + ++void ScColumn::InterpretCellsIfNeeded( SCROW nRow1, SCROW nRow2 ) ++{ ++ if (!GetDoc().ValidRow(nRow1) || !GetDoc().ValidRow(nRow2) || nRow1 > nRow2) ++ return; ++ ++ NeedsInterpretCellInterpreter aFunc; ++ sc::ProcessFormula(maCells.begin(), maCells, nRow1, nRow2, aFunc); ++} ++ + void ScColumn::DeleteContent( SCROW nRow, bool bBroadcast ) + { + sc::CellStoreType::position_type aPos = maCells.position(nRow); +diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx +index b62407916300..dbf6233efa6b 100644 +--- a/sc/source/core/data/document.cxx ++++ b/sc/source/core/data/document.cxx +@@ -3923,6 +3923,23 @@ void ScDocument::InterpretDirtyCells( const ScRangeList& rRanges ) + mpFormulaGroupCxt.reset(); + } + ++void ScDocument::InterpretCellsIfNeeded( const ScRangeList& rRanges ) ++{ ++ for (size_t nPos=0, nRangeCount = rRanges.size(); nPos < nRangeCount; nPos++) ++ { ++ const ScRange& rRange = rRanges[nPos]; ++ for (SCTAB nTab = rRange.aStart.Tab(); nTab <= rRange.aEnd.Tab(); ++nTab) ++ { ++ ScTable* pTab = FetchTable(nTab); ++ if (!pTab) ++ return; ++ ++ pTab->InterpretCellsIfNeeded( ++ rRange.aStart.Col(), rRange.aStart.Row(), rRange.aEnd.Col(), rRange.aEnd.Row()); ++ } ++ } ++} ++ + void ScDocument::AddTableOpFormulaCell( ScFormulaCell* pCell ) + { + if (m_TableOpList.empty()) +diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx +index d613ec905575..a03ab678e7d0 100644 +--- a/sc/source/core/data/table1.cxx ++++ b/sc/source/core/data/table1.cxx +@@ -2536,6 +2536,13 @@ void ScTable::InterpretDirtyCells( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW + aCol[nCol].InterpretDirtyCells(nRow1, nRow2); + } + ++void ScTable::InterpretCellsIfNeeded( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2 ) ++{ ++ nCol2 = ClampToAllocatedColumns(nCol2); ++ for (SCCOL nCol = nCol1; nCol <= nCol2; ++nCol) ++ aCol[nCol].InterpretCellsIfNeeded(nRow1, nRow2); ++} ++ + void ScTable::SetFormulaResults( SCCOL nCol, SCROW nRow, const double* pResults, size_t nLen ) + { + if (!ValidCol(nCol)) +diff --git a/sc/source/ui/view/output2.cxx b/sc/source/ui/view/output2.cxx +index ba4a1b13a795..53ee54fad4c3 100644 +--- a/sc/source/ui/view/output2.cxx ++++ b/sc/source/ui/view/output2.cxx +@@ -1552,6 +1552,11 @@ tools::Rectangle ScOutputData::LayoutStrings(bool bPixelToLogic, bool bPaint, co + const SfxItemSet* pOldCondSet = nullptr; + SvtScriptType nOldScript = SvtScriptType::NONE; + ++ // Try to limit interpreting to only visible cells. Calling e.g. IsValue() ++ // on a formula cell that needs interpreting would call Interpret() ++ // for the entire formula group, which could be large. ++ mpDoc->InterpretCellsIfNeeded( ScRange( nX1, nY1, nTab, nX2, nY2, nTab )); ++ + // alternative pattern instances in case we need to modify the pattern + // before processing the cell value. + std::vector > aAltPatterns; +-- +2.35.3 + diff --git a/libreoffice.changes b/libreoffice.changes index 3ef426b..0442e58 100644 --- a/libreoffice.changes +++ b/libreoffice.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue May 17 09:45:36 UTC 2022 - Andras Timar + +- Fix bsc#1197497 - LO-L3: Loading XLSX with 1M rows is ultra slow (or crashes Calc) + * bsc1197497.patch + ------------------------------------------------------------------- Mon May 2 13:57:03 UTC 2022 - Danilo Spinella diff --git a/libreoffice.spec b/libreoffice.spec index 3cc76d5..4e83682 100644 --- a/libreoffice.spec +++ b/libreoffice.spec @@ -109,6 +109,8 @@ Patch9: fix_math_desktop_file.patch Patch10: fix_gtk_popover_on_3.20.patch # Bug 1192616 - LO-L3: Extraneous/missing lines in table in Impress versus PowerPoint Patch13: bsc1192616.patch +# Bug 1197497 - LO-L3: Loading XLSX with 1M rows is ultra slow (or crashes Calc) +Patch14: bsc1197497.patch # Build with java 8 Patch101: 0001-Revert-java-9-changes.patch # try to save space by using hardlinks @@ -1021,6 +1023,7 @@ Provides %{langname} translations and additional resources (help files, etc.) fo %patch6 -p1 %patch9 -p1 %patch13 -p1 +%patch14 -p1 %if 0%{?suse_version} < 1500 %patch10 -p1 %patch101 -p1