sc/inc/scmatrix.hxx | 6 +
sc/source/core/tool/scmatrix.cxx | 72 +++++++++---
sc/source/filter/oox/workbookhelper.cxx | 2
sc/source/ui/docshell/externalrefmgr.cxx | 180 +++++++++++++++++++++++--------
4 files changed, 199 insertions(+), 61 deletions(-)
New commits:
commit 088ba465d6bb0931c7034d564a3077e21078cb13
Author: Eike Rathke <er...@redhat.com>
Date: Mon Mar 7 13:46:30 2016 +0100
disable means Enable(false), not true.. tdf#98389 related
EnableAdjustHeight(true) lead to the side effect that when clearing the
range of a pivot table during import, formulas in the same rows were
recalculated with then empty data. Through xSheetOp->clearContents() in
PivotTable::finalizeImport() of
sc/source/filter/oox/pivottablebuffer.cxx
In the case of the bugdoc attached to tdf#98389 that lead to all
VLOOKUP() calls resulting in #N/A errors because an empty string was
looked up.
Change-Id: Ie629ce871d255cc871671165c4da974f21041cec
diff --git a/sc/source/filter/oox/workbookhelper.cxx
b/sc/source/filter/oox/workbookhelper.cxx
index ec76d54..c21e471 100644
--- a/sc/source/filter/oox/workbookhelper.cxx
+++ b/sc/source/filter/oox/workbookhelper.cxx
@@ -604,7 +604,7 @@ void WorkbookGlobals::initialize( bool bWorkbookFile )
// #i76026# disable Undo while loading the document
mpDoc->EnableUndo(false);
// #i79826# disable calculating automatic row height while loading the
document
- mpDoc->EnableAdjustHeight(true);
+ mpDoc->EnableAdjustHeight(false);
// disable automatic update of linked sheets and DDE links
mpDoc->EnableExecuteLink(false);
commit 9e60bbdb3aa5f80ca80c9c4fdf7accd12c4a5d1c
Author: Eike Rathke <er...@redhat.com>
Date: Mon Mar 7 13:20:54 2016 +0100
Resolves: tdf#98389 check memory requirement of matrix
Attachment 123203 of tdf#98389 has external references of the form
[1]Data!$1:$1048576
which effectively address the entire cell range of sheet Data and
allocating a matrix for a billion cells attempted to allocate 8GB+xGB of
memory which the system said no-no ...
Some brave soul along the road once decided that a check on the number
of elements wasn't needed anymore and dared to remove it during
refactoring.. This change reintroduces the check and adds it also to
other places that try to instanciate or resize a matrix.
When allocation of a matrix for an external range reference fails it is
checked whether the request is for such nasty entire rows or entire
columns ranges and if so tries to shrink the range to the actual data
area used, which at least in this case helps and works.
Additionally it fixes a long standing bug in the cached area to array
mapping which caused the cache to not be hit but instead the matrix was
aquired over and over again for the same range, causing a serious
performance bottle neck specifically for VLOOKUP and related functions
where the same external range is used in a lot of cells.
Change-Id: I35f67b48e665bf1cc8d672325c32b889f2e05c57
diff --git a/sc/inc/scmatrix.hxx b/sc/inc/scmatrix.hxx
index 64147c2..d82cfa4 100644
--- a/sc/inc/scmatrix.hxx
+++ b/sc/inc/scmatrix.hxx
@@ -164,6 +164,12 @@ public:
#endif
}
+ /** Checks nC or nR for zero and uses GetElementsMax() whether a matrix of
+ the size of nC*nR could be allocated. A zero size (both nC and nR zero)
+ matrix is allowed for later resize.
+ */
+ bool static IsSizeAllocatable( SCSIZE nC, SCSIZE nR );
+
/// Value or boolean.
inline static bool IsValueType( ScMatValType nType )
{
diff --git a/sc/source/core/tool/scmatrix.cxx b/sc/source/core/tool/scmatrix.cxx
index 1a5d012..1477d9d 100644
--- a/sc/source/core/tool/scmatrix.cxx
+++ b/sc/source/core/tool/scmatrix.cxx
@@ -337,14 +337,32 @@ void ScMatrixImpl::Clear()
void ScMatrixImpl::Resize(SCSIZE nC, SCSIZE nR)
{
- maMat.resize(nR, nC);
- maMatFlag.resize(nR, nC);
+ if (ScMatrix::IsSizeAllocatable( nC, nR))
+ {
+ maMat.resize(nR, nC);
+ maMatFlag.resize(nR, nC);
+ }
+ else
+ {
+ // Invalid matrix size, allocate 1x1 matrix with error value.
+ maMat.resize(1, 1, CreateDoubleError( errStackOverflow));
+ maMatFlag.resize(1, 1);
+ }
}
void ScMatrixImpl::Resize(SCSIZE nC, SCSIZE nR, double fVal)
{
- maMat.resize(nR, nC, fVal);
- maMatFlag.resize(nR, nC);
+ if (ScMatrix::IsSizeAllocatable( nC, nR))
+ {
+ maMat.resize(nR, nC, fVal);
+ maMatFlag.resize(nR, nC);
+ }
+ else
+ {
+ // Invalid matrix size, allocate 1x1 matrix with error value.
+ maMat.resize(1, 1, CreateDoubleError( errStackOverflow));
+ maMatFlag.resize(1, 1);
+ }
}
void ScMatrixImpl::SetErrorInterpreter( ScInterpreter* p)
@@ -2356,28 +2374,52 @@ void ScMatrix::DecRef() const
delete this;
}
-ScFullMatrix::ScFullMatrix( SCSIZE nC, SCSIZE nR) :
- ScMatrix(),
- pImpl(new ScMatrixImpl(nC, nR))
+bool ScMatrix::IsSizeAllocatable( SCSIZE nC, SCSIZE nR )
{
SAL_WARN_IF( !nC, "sc", "ScMatrix with 0 columns!");
SAL_WARN_IF( !nR, "sc", "ScMatrix with 0 rows!");
+ // 0-size matrix is valid, it could be resized later.
+ if ((nC && !nR) || (!nC && nR))
+ {
+ SAL_WARN( "sc", "ScMatrix one-dimensional zero: " << nC << " columns *
" << nR << " rows");
+ return false;
+ }
+ if (nC && nR && (nC > (ScMatrix::GetElementsMax() / nR)))
+ {
+ SAL_WARN( "sc", "ScMatrix overflow: " << nC << " columns * " << nR <<
" rows");
+ return false;
+ }
+ return true;
+}
+
+ScFullMatrix::ScFullMatrix( SCSIZE nC, SCSIZE nR) :
+ ScMatrix()
+{
+ if (ScMatrix::IsSizeAllocatable( nC, nR))
+ pImpl.reset( new ScMatrixImpl( nC, nR));
+ else
+ // Invalid matrix size, allocate 1x1 matrix with error value.
+ pImpl.reset( new ScMatrixImpl( 1,1, CreateDoubleError(
errStackOverflow)));
}
ScFullMatrix::ScFullMatrix(SCSIZE nC, SCSIZE nR, double fInitVal) :
- ScMatrix(),
- pImpl(new ScMatrixImpl(nC, nR, fInitVal))
+ ScMatrix()
{
- SAL_WARN_IF( !nC, "sc", "ScMatrix with 0 columns!");
- SAL_WARN_IF( !nR, "sc", "ScMatrix with 0 rows!");
+ if (ScMatrix::IsSizeAllocatable( nC, nR))
+ pImpl.reset( new ScMatrixImpl( nC, nR, fInitVal));
+ else
+ // Invalid matrix size, allocate 1x1 matrix with error value.
+ pImpl.reset( new ScMatrixImpl( 1,1, CreateDoubleError(
errStackOverflow)));
}
ScFullMatrix::ScFullMatrix( size_t nC, size_t nR, const std::vector<double>&
rInitVals ) :
- ScMatrix(),
- pImpl(new ScMatrixImpl(nC, nR, rInitVals))
+ ScMatrix()
{
- SAL_WARN_IF( !nC, "sc", "ScMatrix with 0 columns!");
- SAL_WARN_IF( !nR, "sc", "ScMatrix with 0 rows!");
+ if (ScMatrix::IsSizeAllocatable( nC, nR))
+ pImpl.reset( new ScMatrixImpl( nC, nR, rInitVals));
+ else
+ // Invalid matrix size, allocate 1x1 matrix with error value.
+ pImpl.reset( new ScMatrixImpl( 1,1, CreateDoubleError(
errStackOverflow)));
}
ScFullMatrix::~ScFullMatrix()
diff --git a/sc/source/ui/docshell/externalrefmgr.cxx
b/sc/source/ui/docshell/externalrefmgr.cxx
index 7f053de..28c8fb0 100644
--- a/sc/source/ui/docshell/externalrefmgr.cxx
+++ b/sc/source/ui/docshell/externalrefmgr.cxx
@@ -580,9 +580,9 @@ ScExternalRefCache::TokenArrayRef
ScExternalRefCache::getCellRangeData(
const ScAddress& s = rRange.aStart;
const ScAddress& e = rRange.aEnd;
- SCTAB nTab1 = s.Tab(), nTab2 = e.Tab();
- SCCOL nCol1 = s.Col(), nCol2 = e.Col();
- SCROW nRow1 = s.Row(), nRow2 = e.Row();
+ const SCTAB nTab1 = s.Tab(), nTab2 = e.Tab();
+ const SCCOL nCol1 = s.Col(), nCol2 = e.Col();
+ const SCROW nRow1 = s.Row(), nRow2 = e.Row();
// Make sure I have all the tables cached.
size_t nTabFirstId = itrTabId->second;
@@ -616,58 +616,120 @@ ScExternalRefCache::TokenArrayRef
ScExternalRefCache::getCellRangeData(
return TokenArrayRef();
}
- ScMatrixRef xMat = new ScFullMatrix(
- static_cast<SCSIZE>(nDataCol2-nDataCol1+1),
static_cast<SCSIZE>(nDataRow2-nDataRow1+1));
+ SCSIZE nMatrixColumns = static_cast<SCSIZE>(nDataCol2-nDataCol1+1);
+ SCSIZE nMatrixRows = static_cast<SCSIZE>(nDataRow2-nDataRow1+1);
+ ScMatrixRef xMat = new ScFullMatrix( nMatrixColumns, nMatrixRows);
- // Only fill non-empty cells, for better performance.
+ // Needed in shrink and fill.
vector<SCROW> aRows;
pTab->getAllRows(aRows, nDataRow1, nDataRow2);
- for (vector<SCROW>::const_iterator itr = aRows.begin(), itrEnd =
aRows.end(); itr != itrEnd; ++itr)
+ bool bFill = true;
+
+ // Check if size could be allocated and if not skip the fill, there's
+ // one error element instead. But retry first with the actual data area
+ // if that is smaller than the original range, which works for most
+ // functions just not some that operate/compare with the original size
+ // and expect empty values in non-data areas.
+ // Restrict this though to ranges of entire columns or rows, other
+ // ranges might be on purpose. (Other special cases to handle?)
+ /* TODO: sparse matrix could help */
+ SCSIZE nMatCols, nMatRows;
+ xMat->GetDimensions( nMatCols, nMatRows);
+ if (nMatCols != nMatrixColumns || nMatRows != nMatrixRows)
{
- SCROW nRow = *itr;
- vector<SCCOL> aCols;
- pTab->getAllCols(nRow, aCols, nDataCol1, nDataCol2);
- for (vector<SCCOL>::const_iterator itrCol = aCols.begin(),
itrColEnd = aCols.end(); itrCol != itrColEnd; ++itrCol)
+ bFill = false;
+ if (aRows.empty())
{
- SCCOL nCol = *itrCol;
- TokenRef pToken = pTab->getCell(nCol, nRow);
- if (!pToken)
- // This should never happen!
- return TokenArrayRef();
-
- SCSIZE nC = nCol - nDataCol1, nR = nRow - nDataRow1;
- switch (pToken->GetType())
+ // There's no data at all. Set the one matrix element to empty
+ // for column-repeated and row-repeated access.
+ xMat->PutEmpty(0,0);
+ }
+ else if ((nCol1 == 0 && nCol2 == MAXCOL) || (nRow1 == 0 && nRow2
== MAXROW))
+ {
+ nDataRow1 = aRows.front();
+ nDataRow2 = aRows.back();
+ SCCOL nMinCol = std::numeric_limits<SCCOL>::max();
+ SCCOL nMaxCol = std::numeric_limits<SCCOL>::min();
+ for (vector<SCROW>::const_iterator itr = aRows.begin(), itrEnd
= aRows.end(); itr != itrEnd; ++itr)
{
- case svDouble:
- xMat->PutDouble(pToken->GetDouble(), nC, nR);
- break;
- case svString:
- xMat->PutString(pToken->GetString(), nC, nR);
- break;
- default:
- ;
+ vector<SCCOL> aCols;
+ pTab->getAllCols(*itr, aCols, nDataCol1, nDataCol2);
+ if (!aCols.empty())
+ {
+ nMinCol = std::min( nMinCol, aCols.front());
+ nMaxCol = std::max( nMaxCol, aCols.back());
+ }
+ }
+
+ if (nMinCol <= nMaxCol &&
((static_cast<SCSIZE>(nMaxCol-nMinCol+1) < nMatrixColumns) ||
+ (static_cast<SCSIZE>(nDataRow2-nDataRow1+1) <
nMatrixRows)))
+ {
+ nMatrixColumns = static_cast<SCSIZE>(nMaxCol-nMinCol+1);
+ nMatrixRows = static_cast<SCSIZE>(nDataRow2-nDataRow1+1);
+ xMat = new ScFullMatrix( nMatrixColumns, nMatrixRows);
+ xMat->GetDimensions( nMatCols, nMatRows);
+ if (nMatCols == nMatrixColumns && nMatRows == nMatrixRows)
+ {
+ nDataCol1 = nMinCol;
+ nDataCol2 = nMaxCol;
+ bFill = true;
+ }
}
}
}
- if (!bFirstTab)
- pArray->AddOpCode(ocSep);
+ if (bFill)
+ {
+ // Only fill non-empty cells, for better performance.
+ for (vector<SCROW>::const_iterator itr = aRows.begin(), itrEnd =
aRows.end(); itr != itrEnd; ++itr)
+ {
+ SCROW nRow = *itr;
+ vector<SCCOL> aCols;
+ pTab->getAllCols(nRow, aCols, nDataCol1, nDataCol2);
+ for (vector<SCCOL>::const_iterator itrCol = aCols.begin(),
itrColEnd = aCols.end(); itrCol != itrColEnd; ++itrCol)
+ {
+ SCCOL nCol = *itrCol;
+ TokenRef pToken = pTab->getCell(nCol, nRow);
+ if (!pToken)
+ // This should never happen!
+ return TokenArrayRef();
+
+ SCSIZE nC = nCol - nDataCol1, nR = nRow - nDataRow1;
+ switch (pToken->GetType())
+ {
+ case svDouble:
+ xMat->PutDouble(pToken->GetDouble(), nC, nR);
+ break;
+ case svString:
+ xMat->PutString(pToken->GetString(), nC, nR);
+ break;
+ default:
+ ;
+ }
+ }
+ }
- ScMatrixToken aToken(xMat);
- if (!pArray)
- pArray.reset(new ScTokenArray);
- pArray->AddToken(aToken);
+ if (!bFirstTab)
+ pArray->AddOpCode(ocSep);
- bFirstTab = false;
+ ScMatrixToken aToken(xMat);
+ if (!pArray)
+ pArray.reset(new ScTokenArray);
+ pArray->AddToken(aToken);
- if (!pNewRange)
- pNewRange.reset(new ScRange(nDataCol1, nDataRow1, 0, nDataCol2,
nDataRow2, 0));
- else
- pNewRange->ExtendTo(ScRange(nDataCol1, nDataRow1, 0, nDataCol2,
nDataRow2, 0));
+ bFirstTab = false;
+
+ if (!pNewRange)
+ pNewRange.reset(new ScRange(nDataCol1, nDataRow1, nTab,
nDataCol2, nDataRow2, nTab));
+ else
+ pNewRange->ExtendTo(ScRange(nDataCol1, nDataRow1, nTab,
nDataCol2, nDataRow2, nTab));
+ }
}
- if (pNewRange)
+ rDoc.maRangeArrays.insert( RangeArrayMap::value_type(aCacheRange, pArray));
+ if (pNewRange && *pNewRange != aCacheRange)
rDoc.maRangeArrays.insert( RangeArrayMap::value_type(*pNewRange,
pArray));
+
return pArray;
}
@@ -1453,9 +1515,9 @@ static std::unique_ptr<ScTokenArray> convertToTokenArray(
ScAddress& s = rRange.aStart;
ScAddress& e = rRange.aEnd;
- SCTAB nTab1 = s.Tab(), nTab2 = e.Tab();
- SCCOL nCol1 = s.Col(), nCol2 = e.Col();
- SCROW nRow1 = s.Row(), nRow2 = e.Row();
+ const SCTAB nTab1 = s.Tab(), nTab2 = e.Tab();
+ const SCCOL nCol1 = s.Col(), nCol2 = e.Col();
+ const SCROW nRow1 = s.Row(), nRow2 = e.Row();
if (nTab2 != nTab1)
// For now, we don't support multi-sheet ranges intentionally because
@@ -1488,10 +1550,38 @@ static std::unique_ptr<ScTokenArray>
convertToTokenArray(
else
pUsedRange.reset(new ScRange(nDataCol1, nDataRow1, 0, nDataCol2,
nDataRow2, 0));
- ScMatrixRef xMat = new ScFullMatrix(
- static_cast<SCSIZE>(nCol2-nCol1+1),
static_cast<SCSIZE>(nRow2-nRow1+1));
+ SCSIZE nMatrixColumns = static_cast<SCSIZE>(nCol2-nCol1+1);
+ SCSIZE nMatrixRows = static_cast<SCSIZE>(nRow2-nRow1+1);
+ ScMatrixRef xMat = new ScFullMatrix( nMatrixColumns, nMatrixRows);
+
+ // Check if size could be allocated and if not skip the fill, there's
+ // one error element instead. But retry first with the actual data area
+ // if that is smaller than the original range, which works for most
+ // functions just not some that operate/compare with the original size
+ // and expect empty values in non-data areas.
+ // Restrict this though to ranges of entire columns or rows, other
+ // ranges might be on purpose. (Other special cases to handle?)
+ /* TODO: sparse matrix could help */
+ SCSIZE nMatCols, nMatRows;
+ xMat->GetDimensions( nMatCols, nMatRows);
+ if (nMatCols == nMatrixColumns && nMatRows == nMatrixRows)
+ {
+ pSrcDoc->FillMatrix(*xMat, nTab, nCol1, nRow1, nCol2, nRow2,
&pHostDoc->GetSharedStringPool());
+ }
+ else if ((nCol1 == 0 && nCol2 == MAXCOL) || (nRow1 == 0 && nRow2 ==
MAXROW))
+ {
+ if ((static_cast<SCSIZE>(nDataCol2-nDataCol1+1) < nMatrixColumns)
||
+ (static_cast<SCSIZE>(nDataRow2-nDataRow1+1) < nMatrixRows))
+ {
+ nMatrixColumns = static_cast<SCSIZE>(nDataCol2-nDataCol1+1);
+ nMatrixRows = static_cast<SCSIZE>(nDataRow2-nDataRow1+1);
+ xMat = new ScFullMatrix( nMatrixColumns, nMatrixRows);
+ xMat->GetDimensions( nMatCols, nMatRows);
+ if (nMatCols == nMatrixColumns && nMatRows == nMatrixRows)
+ pSrcDoc->FillMatrix(*xMat, nTab, nDataCol1, nDataRow1,
nDataCol2, nDataRow2, &pHostDoc->GetSharedStringPool());
+ }
+ }
- pSrcDoc->FillMatrix(*xMat, nTab, nCol1, nRow1, nCol2, nRow2,
&pHostDoc->GetSharedStringPool());
if (!bFirstTab)
pArray->AddOpCode(ocSep);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
