summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTor Lillqvist <tml@collabora.com>2015-03-10 09:04:57 +0200
committerTor Lillqvist <tml@collabora.com>2015-03-10 09:28:09 +0200
commitddc1f7d9a816e2cc970d48d2ccc2c0cd256e6e03 (patch)
treed19552d0624b5a96aa120c5551729f591c9dc1c6
parentbb0c05cebed131805d04919fac2b83030087451b (diff)
Fix issue in re-use of ScTokenArray objects from a TokenPool
The TokenPool::operator[] is used to initialise and take into use an object from the pool. Which is a fascinating thing as such and probably not entirely in good style. Anyway, the objects in the pool are of type ScTokenArray, a class derived from FormulaTokenArray. The operator[] called the FormulaTokenArray::Clear() function to initialise the object. This left the fields added in ScTokenArray uninitialised, having whatever value the previous use of the object had set. Which of course is bad. In practice, this showed up in the handling of formulas in the .xls input filter. If an earlier (or the first?) formula had happened to be one for which we don't want to use OpenCL, the meVectorState of its ScTokenArray object in the pool had been set to FormulaVectorDisabled. When the same pool object was later re-used for another formula, it kept that same meVectorState, even if there was no reason to. Thus formula groups that should have been OpenCL accelerated weren't. This can have a significant impact on performance of document loading and recalculation for large documents. I added a function to ScTokenArray to clear (initialise) such an object, both the FormulaTokenArray part and the ScTokenArray-specific part, and call that instead. This fixes the issue. I named the added function ClearScTokenArray() to make it clear that it is a separate function. Sure, possibly Clear() should be made into a virtual of FormulaTokenArry and overridden in ScTokenArray, and the overriding Clear() would first call the base class's Clear(). But I can't be sure that there aren't other calls of FormulaTokenArray::Clear() that *must* mean the base class one. Better safe than sorry. And of course, I did *not* want to name the function in ScTokenArray also "Clear()", like in the base class, without it being virtual. That is horrible style in my opinion, even if there certainly is precedence for such even in the very same classes, i.e. the Clone() function... Change-Id: I0e0e13e5ca705603005a1e0a46866f095cd2ac4d
-rw-r--r--sc/inc/tokenarray.hxx1
-rw-r--r--sc/source/core/tool/token.cxx6
-rw-r--r--sc/source/filter/inc/tokstack.hxx2
3 files changed, 8 insertions, 1 deletions
diff --git a/sc/inc/tokenarray.hxx b/sc/inc/tokenarray.hxx
index 40687455bac9..0d3fa0245f50 100644
--- a/sc/inc/tokenarray.hxx
+++ b/sc/inc/tokenarray.hxx
@@ -58,6 +58,7 @@ public:
/// Assignment with references to FormulaToken entries (not copied!)
ScTokenArray( const ScTokenArray& );
virtual ~ScTokenArray();
+ void ClearScTokenArray();
ScTokenArray* Clone() const; /// True copy!
// An estimate of the number of cells referenced by the token array
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index 55a060135cb7..203f3783f31e 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -1651,6 +1651,12 @@ ScTokenArray& ScTokenArray::operator=( const ScTokenArray& rArr )
return *this;
}
+void ScTokenArray::ClearScTokenArray()
+{
+ Clear();
+ meVectorState = FormulaVectorEnabled;
+}
+
ScTokenArray* ScTokenArray::Clone() const
{
ScTokenArray* p = new ScTokenArray();
diff --git a/sc/source/filter/inc/tokstack.hxx b/sc/source/filter/inc/tokstack.hxx
index 7513d481b178..9e458d7b0730 100644
--- a/sc/source/filter/inc/tokstack.hxx
+++ b/sc/source/filter/inc/tokstack.hxx
@@ -362,7 +362,7 @@ inline const TokenId TokenPool::LastId( void ) const
const inline ScTokenArray* TokenPool::operator []( const TokenId& rId )
{
- pScToken->Clear();
+ pScToken->ClearScTokenArray();
if( rId )
{//...only if rId > 0!