tdf#68016 Speed up ScChart2DataSequence by caching addresses

The lookup of getNumberFormatKeyByIndex() is sped up by storing the
addresses into the cached data array m_aDataArray.

The existing cache invalidating strategy should hold since the
cache was already storing information about hidden fields and
ranges, which is the information which affects the addresses.

Also: Change data type of m_aDataArray from std::list to std::vector to
allow index-based access.

Also: Change for-loops over m_aDataArray to range-based loops with auto
variables to make them more readable

Change-Id: I9a5038892a384e7d5e72556a52faaf98b475a839
Reviewed-on: https://gerrit.libreoffice.org/16485
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Eike Rathke <erack@redhat.com>
Reviewed-by: Markus Mohrhard <markus.mohrhard@googlemail.com>

2 files changed, 45 insertions, 72 deletions

diff --git a/sc/inc/chart2uno.hxx b/sc/inc/chart2uno.hxx
index 03e988c0400b..68387a608f53 100644
--- a/sc/inc/chart2uno.hxx
+++ b/sc/inc/chart2uno.hxx
@@ -262,6 +262,10 @@ public:
     virtual ::com::sun::star::uno::Sequence< OUString >
         SAL_CALL generateLabel(::com::sun::star::chart2::data::LabelOrigin nOrigin)
         throw (::com::sun::star::uno::RuntimeException, std::exception) SAL_OVERRIDE;
+
+    /** Get the number format key for the n-th data entry
+     * If nIndex == -1, then you will get the number format key for the first non-empty entry
+     */
     virtual ::sal_Int32 SAL_CALL getNumberFormatKeyByIndex( ::sal_Int32 nIndex )
         throw (::com::sun::star::lang::IndexOutOfBoundsException,
                ::com::sun::star::uno::RuntimeException,
@@ -416,6 +420,7 @@ private:
         double              mfValue;
         OUString     maString;
         bool                mbIsValue;
+        ScAddress   mAddress;
         Item();
     };
 
@@ -431,7 +436,8 @@ private:
         ScChart2DataSequence& mrParent;
     };
 
-    ::std::list<Item>           m_aDataArray;
+    /** This vector contains the cached data which was calculated with BuildDataCache(). */
+    std::vector<Item>           m_aDataArray;
 
     /**
      * Cached data for getData.  We may also need to cache data for the
diff --git a/sc/source/ui/unoobj/chart2uno.cxx b/sc/source/ui/unoobj/chart2uno.cxx
index e0793a22ed21..5373bf812308 100644
--- a/sc/source/ui/unoobj/chart2uno.cxx
+++ b/sc/source/ui/unoobj/chart2uno.cxx
@@ -2642,6 +2642,8 @@ void ScChart2DataSequence::BuildDataCache()
                                 ; // do nothing
                         }
 
+                        aItem.mAddress = ScAddress(nCol, nRow, nTab);
+
                         m_aDataArray.push_back(aItem);
                         ++nDataCount;
                     }
@@ -2808,7 +2810,7 @@ void ScChart2DataSequence::CopyData(const ScChart2DataSequence& r)
         return;
     }
 
-    list<Item> aDataArray(r.m_aDataArray);
+    std::vector<Item> aDataArray(r.m_aDataArray);
     m_aDataArray.swap(aDataArray);
 
     m_aHiddenValues = r.m_aHiddenValues;
@@ -3023,13 +3025,13 @@ uno::Sequence< uno::Any> SAL_CALL ScChart2DataSequence::getData()
         sal_Int32 nCount = m_aDataArray.size();
         m_aMixedDataCache.realloc(nCount);
         uno::Any* pArr = m_aMixedDataCache.getArray();
-        ::std::list<Item>::const_iterator itr = m_aDataArray.begin(), itrEnd = m_aDataArray.end();
-        for (; itr != itrEnd; ++itr, ++pArr)
+        for (const Item &rItem : m_aDataArray)
         {
-            if (itr->mbIsValue)
-                *pArr <<= itr->mfValue;
+            if (rItem.mbIsValue)
+                *pArr <<= rItem.mfValue;
             else
-                *pArr <<= itr->maString;
+                *pArr <<= rItem.maString;
+            ++pArr;
         }
     }
     return m_aMixedDataCache;
@@ -3052,9 +3054,11 @@ uno::Sequence< double > SAL_CALL ScChart2DataSequence::getNumericalData()
     sal_Int32 nCount = m_aDataArray.size();
     uno::Sequence<double> aSeq(nCount);
     double* pArr = aSeq.getArray();
-    ::std::list<Item>::const_iterator itr = m_aDataArray.begin(), itrEnd = m_aDataArray.end();
-    for (; itr != itrEnd; ++itr, ++pArr)
-        *pArr = itr->mbIsValue ? itr->mfValue : fNAN;
+    for (const Item& rItem : m_aDataArray)
+    {
+        *pArr = rItem.mbIsValue ? rItem.mfValue : fNAN;
+        ++pArr;
+    }
 
     return aSeq;
 }
@@ -3076,9 +3080,11 @@ uno::Sequence< OUString > SAL_CALL ScChart2DataSequence::getTextualData()
     {
         aSeq =  uno::Sequence<OUString>(nCount);
         OUString* pArr = aSeq.getArray();
-        ::std::list<Item>::const_iterator itr = m_aDataArray.begin(), itrEnd = m_aDataArray.end();
-        for(; itr != itrEnd; ++itr, ++pArr)
-            *pArr = itr->maString;
+        for (const Item& rItem : m_aDataArray)
+        {
+            *pArr = rItem.maString;
+            ++pArr;
+        }
     }
     else if ( m_pTokens.get() && m_pTokens->front() )
     {
@@ -3268,73 +3274,34 @@ sal_uLong getDisplayNumberFormat(ScDocument* pDoc, const ScAddress& rPos)
 ::sal_Int32 SAL_CALL ScChart2DataSequence::getNumberFormatKeyByIndex( ::sal_Int32 nIndex )
     throw (lang::IndexOutOfBoundsException, uno::RuntimeException, std::exception)
 {
-    // index -1 means a heuristic value for the entire sequence
-    bool bGetSeriesFormat = (nIndex == -1);
-
     SolarMutexGuard aGuard;
-    if ( !m_pDocument || !m_pTokens.get())
-        return 0;
-
-    // TODO: Handle external references too.
-
-    sal_Int32 nCount = 0;
+    BuildDataCache();
 
-    ScRangeList aRanges;
-    ScRefTokenHelper::getRangeListFromTokens(aRanges, *m_pTokens, ScAddress());
-    for (size_t i = 0, n = aRanges.size(); i < n; ++i)
+    if (nIndex == -1)
     {
-        ScRange* p = aRanges[i];
-        for (SCTAB nTab = p->aStart.Tab(); nTab <= p->aEnd.Tab(); ++nTab)
+        // return format of first non-empty cell
+        // TODO: use nicer heuristic
+        for (const Item& rItem : m_aDataArray)
         {
-            for (SCCOL nCol = p->aStart.Col(); nCol <= p->aEnd.Col(); ++nCol)
+            ScRefCellValue aCell;
+            aCell.assign(*m_pDocument, rItem.mAddress);
+            if (!aCell.isEmpty())
             {
-                if (!m_bIncludeHiddenCells)
-                {
-                    // Skip hidden columns.
-                    SCCOL nLastCol = -1;
-                    bool bColHidden = m_pDocument->ColHidden(nCol, nTab, NULL, &nLastCol);
-                    if (bColHidden)
-                    {
-                        nCol = nLastCol;
-                        continue;
-                    }
-                }
-
-                for (SCROW nRow = p->aStart.Row(); nRow <= p->aEnd.Row(); ++nRow)
-                {
-                    if (!m_bIncludeHiddenCells)
-                    {
-                        // Skip hidden rows.
-                        SCROW nLastRow = -1;
-                        bool bRowHidden = m_pDocument->RowHidden(nRow, nTab, NULL, &nLastRow);
-                        if (bRowHidden)
-                        {
-                            nRow = nLastRow;
-                            continue;
-                        }
-                    }
-
-                    ScAddress aPos(nCol, nRow, nTab);
-
-                    if( bGetSeriesFormat )
-                    {
-                        // TODO: use nicer heuristic
-                        // return format of first non-empty cell
-                        ScRefCellValue aCell;
-                        aCell.assign(*m_pDocument, aPos);
-                        if (!aCell.isEmpty())
-                            return static_cast<sal_Int32>(getDisplayNumberFormat(m_pDocument, aPos));
-                    }
-                    else if( nCount == nIndex )
-                    {
-                        return static_cast<sal_Int32>(getDisplayNumberFormat(m_pDocument, aPos));
-                    }
-                    ++nCount;
-                }
+                return static_cast<sal_Int32>(getDisplayNumberFormat(m_pDocument, rItem.mAddress));
             }
         }
+
+       // we could not find a non-empty cell
+       return 0;
     }
-    return 0;
+
+    if (nIndex < 0 || nIndex >= static_cast<sal_Int32>(m_aDataArray.size()))
+    {
+        SAL_WARN("sc.ui", "Passed invalid index to getNumberFormatKeyByIndex(). Will return default value '0'.");
+        return 0;
+    }
+
+    return static_cast<sal_Int32>(getDisplayNumberFormat(m_pDocument, m_aDataArray.at(nIndex).mAddress));
 }
 
 // XCloneable ================================================================