[Libreoffice-commits] core.git: sc/inc sc/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Thu Nov 22 20:25:21 UTC 2018


 sc/inc/lookupcache.hxx              |    8 ++++
 sc/source/core/tool/interpr1.cxx    |   58 ++++++++++++++++++++++++++++++++++++
 sc/source/core/tool/lookupcache.cxx |   13 ++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)

New commits:
commit 9e55c8c7ffc43004ca314edcd4712dcc29adf933
Author:     Serge Krot <Serge.Krot at cib.de>
AuthorDate: Wed Oct 31 22:11:31 2018 +0100
Commit:     Eike Rathke <erack at redhat.com>
CommitDate: Thu Nov 22 21:24:55 2018 +0100

    tdf#121052 sc: avoid multiple empty value lookups in ranges
    
    Change-Id: I7759aef51af2f400f3f5ec69854fd9133e845f49
    Reviewed-on: https://gerrit.libreoffice.org/62712
    Tested-by: Jenkins
    Reviewed-by: Eike Rathke <erack at redhat.com>

diff --git a/sc/inc/lookupcache.hxx b/sc/inc/lookupcache.hxx
index 3a2be452a2ee..ea657c67d787 100644
--- a/sc/inc/lookupcache.hxx
+++ b/sc/inc/lookupcache.hxx
@@ -104,6 +104,10 @@ public:
                 (mbString ? (*mpStr == *r.mpStr) : (mfVal == r.mfVal));
         }
 
+        bool isEmptyStringQuery() const
+        {
+            return (getQueryOp() == QueryOp::EQUAL) && mbString && mpStr && mpStr->isEmpty();
+        }
     };
 
     /// MUST be new'd because Notify() deletes.
@@ -112,11 +116,13 @@ public:
     /// Remove from document structure and delete (!) cache on modify hint.
     virtual void Notify( const SfxHint& rHint ) override;
 
-    /// @returns document address in o_rAddress if Result==FOUND
+    /// @returns document address in o_rResultAddress if Result==FOUND
             Result          lookup( ScAddress & o_rResultAddress,
                                     const QueryCriteria & rCriteria,
                                     const ScAddress & rQueryAddress ) const;
 
+            SCROW           lookup( const QueryCriteria & rCriteria ) const;
+
     /** Insert query and result.
         @param bAvailable
             Pass sal_False if the search didn't deliver a result. A subsequent
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index 14c57288ecef..b1bca830b582 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -9763,6 +9763,46 @@ static bool lcl_LookupQuery( ScAddress & o_rResultPos, ScDocument * pDoc, const
     return bFound;
 }
 
+// tdf#121052:
+// =VLOOKUP(SearchCriterion; RangeArray; Index; Sorted)
+//  [SearchCriterion] is the value searched for in the first column of the array.
+//  [RangeArray] is the reference, which is to comprise at least two columns.
+//  [Index] is the number of the column in the array that contains the value to be returned. The first column has the number 1.
+//
+// lcl_getPrevRowWithEmptyValueLookup() performs following checks:
+// - if value referenced by [SearchCriterion] is empty
+// - and if we run query with "exact match" mode (i.e. VLOOKUP)
+// - and if we already have the same lookup done before but for another row
+//   which is also had empty [SearchCriterion]
+//
+// then
+//   we could say, that for current row we could reuse results of the cached call which was done for the row2
+//   In this case we return row index, which is >= 0.
+//
+// Elsewhere
+//   -1 is returned, which will lead to default behavior =>
+//   complete lookup will be done in RangeArray inside lcl_LookupQuery() method.
+//
+// This method was added only for speed up to avoid several useless complete
+// lookups inside [RangeArray] for searching empty strings.
+//
+static SCROW lcl_getPrevRowWithEmptyValueLookup(const ScLookupCache& rCache, const ScLookupCache::QueryCriteria& aCriteria, const ScQueryParam & rParam)
+{
+    // is search with equal match?
+    if (! aCriteria.isEmptyStringQuery())
+        return -1; // not found
+
+    // is lookup value empty?
+    const ScQueryEntry& rEntry = rParam.GetEntry(0);
+    const ScQueryEntry::Item& rItem = rEntry.GetQueryItem();
+    if (! rItem.maString.getString().isEmpty())
+        return -1; // not found
+
+    // try to find the row index for which we have already performed lookup
+    // and have some result of it inside cache
+    return rCache.lookup( aCriteria );
+}
+
 bool ScInterpreter::LookupQueryWithCache( ScAddress & o_rResultPos,
         const ScQueryParam & rParam ) const
 {
@@ -9786,6 +9826,24 @@ bool ScInterpreter::LookupQueryWithCache( ScAddress & o_rResultPos,
         ScLookupCache::QueryCriteria aCriteria( rEntry);
         ScLookupCache::Result eCacheResult = rCache.lookup( o_rResultPos,
                 aCriteria, aPos);
+
+        // tdf#121052: Slow load of cells with VLOOKUP with references to empty cells
+        // This check was added only for speed up to avoid several useless complete
+        // lookups inside [RangeArray] for searching empty strings.
+        if (eCacheResult == ScLookupCache::NOT_CACHED)
+        {
+            const SCROW nPrevRowWithEmptyValueLookup = lcl_getPrevRowWithEmptyValueLookup(rCache, aCriteria, rParam);
+            if (nPrevRowWithEmptyValueLookup >= 0)
+            {
+                // make the same lookup using cache with different row index
+                // (this lookup was already cached)
+                ScAddress aPosPrev(aPos);
+                aPosPrev.SetRow(nPrevRowWithEmptyValueLookup);
+
+                eCacheResult = rCache.lookup( o_rResultPos, aCriteria, aPosPrev );
+            }
+        }
+
         switch (eCacheResult)
         {
             case ScLookupCache::NOT_CACHED :
diff --git a/sc/source/core/tool/lookupcache.cxx b/sc/source/core/tool/lookupcache.cxx
index 9522b46678fe..5e7890aa2421 100644
--- a/sc/source/core/tool/lookupcache.cxx
+++ b/sc/source/core/tool/lookupcache.cxx
@@ -95,6 +95,19 @@ ScLookupCache::Result ScLookupCache::lookup( ScAddress & o_rResultAddress,
     return FOUND;
 }
 
+SCROW ScLookupCache::lookup( const QueryCriteria & rCriteria ) const
+{
+    // try to find the row index for which we have already performed lookup
+    for (auto it = maQueryMap.begin(); it != maQueryMap.end(); ++it)
+    {
+        if (it->second.maCriteria == rCriteria)
+            return it->first.mnRow;
+    }
+
+    // not found
+    return -1;
+}
+
 bool ScLookupCache::insert( const ScAddress & rResultAddress,
         const QueryCriteria & rCriteria, const ScAddress & rQueryAddress,
         const bool bAvailable )


More information about the Libreoffice-commits mailing list