[PATCH] [REVIEW:3-6] [URGENT] horrible performance regression

Please cherry-pick 0cda6605844ef68e45db7a7c05cc4d09ef2bc49a
(http://cgit.freedesktop.org/libreoffice/core/commit/?id=0cda6605844ef68e45db7a7c05cc4d09ef2bc49a
 and patch also attached)
to libreoffice-3-6 in time for rc2. This would allow me to get some
Base testers in 3.6 before release (as it is, embedded HSQLDB is so
slow as to be unusable).

I'll make a backport to libreoffice-3-5 later this week or next
week, now I have to run to catch a train (that patch most probably
won't apply to libreoffice-3-5.) There are other performance problems
to clean up (fdo#52170, fdo#51976), but I think they are far more
minor that this one.


Some explanation on the commit message: The structure is that a
RowSetCache typically wraps a KeySet. RowSetCache caches a window of
something like 20 or 100 rows, the underlying KeySet caches only the
current row. But RowSetCache keeps its notion of current row and the
wrapped KeySet's notion of current row synchronised, so it is
important that when RowSetCache moves the KeySet's current row, but
already has the data in its own cache, the KeySet does not refetch the
row.


fdo#51239 refresh row lazily (when data is requested)

This avoids fetching data that will not be requested when the "cursor"
is only moved and no data requested. That is typically what
RowSetCache does when its own cursor is moved within its window.

This basically makes the whole {next,previous,absolute,...}_checked
story obsolete, by basically always being as fast as the
i_bFetchRow==false case, but in a safer way.

Change-Id: I89eaf277069736b3077bde8b45325929db290f2d


Thanks folks!


-- 
Lionel

From 0cda6605844ef68e45db7a7c05cc4d09ef2bc49a Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lionel@mamane.lu>
Date: Mon, 16 Jul 2012 23:58:18 +0200
Subject: [PATCH] fdo#51239 refresh row lazily (when data is requested)

This avoids fetching data that will not be requested when the "cursor" is only moved and no data 
requested. That is typically what RowSetCache does when its own cursor is moved within its window.

This basically makes the whole {next,previous,absolute,...}_checked story obsolete, by basically 
always being as fast as the i_bFetchRow==false case, but in a safer way.

Change-Id: I89eaf277069736b3077bde8b45325929db290f2d
---
 .../source/drivers/jdbc/PreparedStatement.cxx      |    1 +
 dbaccess/source/core/api/KeySet.cxx                |   83 ++++++++++----------
 dbaccess/source/core/api/KeySet.hxx                |    1 +
 3 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/connectivity/source/drivers/jdbc/PreparedStatement.cxx 
b/connectivity/source/drivers/jdbc/PreparedStatement.cxx
index 8f90a1c..97dea62 100644
--- a/connectivity/source/drivers/jdbc/PreparedStatement.cxx
+++ b/connectivity/source/drivers/jdbc/PreparedStatement.cxx
@@ -150,6 +150,7 @@ void SAL_CALL java_sql_PreparedStatement::setString( sal_Int32 parameterIndex, c
 
 ::com::sun::star::uno::Reference< ::com::sun::star::sdbc::XResultSet > SAL_CALL 
java_sql_PreparedStatement::executeQuery(  ) throw(::com::sun::star::sdbc::SQLException, 
::com::sun::star::uno::RuntimeException)
 {
+    std::cerr << ::rtl::OUStringToOString( this->m_sSqlStatement, RTL_TEXTENCODING_UTF8 ).getStr() 
<< std::endl;
     m_aLogger.log( LogLevel::FINE, STR_LOG_EXECUTING_PREPARED_QUERY );
     ::osl::MutexGuard aGuard( m_aMutex );
     checkDisposed(java_sql_Statement_BASE::rBHelper.bDisposed);
diff --git a/dbaccess/source/core/api/KeySet.cxx b/dbaccess/source/core/api/KeySet.cxx
index 2709b2e..d5608ef 100644
--- a/dbaccess/source/core/api/KeySet.cxx
+++ b/dbaccess/source/core/api/KeySet.cxx
@@ -398,8 +398,7 @@ sal_Bool SAL_CALL OKeySet::moveToBookmark( const Any& bookmark ) throw(SQLExcept
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", 
"OKeySet::moveToBookmark" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     m_aKeyIter = m_aKeyMap.find(::comphelper::getINT32(bookmark));
-    if (m_aKeyIter != m_aKeyMap.end())
-        refreshRow();
+    invalidateRow();
     return m_aKeyIter != m_aKeyMap.end();
 }
 
@@ -1140,7 +1139,7 @@ sal_Bool SAL_CALL OKeySet::next(  ) throw(SQLException, RuntimeException)
         }
     }
 
-    refreshRow();
+    invalidateRow();
     return !isAfterLast();
 }
 
@@ -1207,7 +1206,7 @@ sal_Bool SAL_CALL OKeySet::first(  ) throw(SQLException, RuntimeException)
         }
     }
     else
-        refreshRow();
+        invalidateRow();
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
 }
 
@@ -1216,7 +1215,7 @@ sal_Bool SAL_CALL OKeySet::last(  ) throw(SQLException, RuntimeException)
     return last_checked(sal_True);
 }
 
-sal_Bool OKeySet::last_checked( sal_Bool i_bFetchRow)
+sal_Bool OKeySet::last_checked( sal_Bool /* i_bFetchRow */ )
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", 
"OKeySet::last_checked" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
@@ -1226,10 +1225,7 @@ sal_Bool OKeySet::last_checked( sal_Bool i_bFetchRow)
     --m_aKeyIter;
     if ( !fetchedRow )
     {
-        if ( i_bFetchRow )
-            refreshRow();
-        else
-            invalidateRow();
+        invalidateRow();
     }
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
 }
@@ -1247,7 +1243,7 @@ sal_Bool SAL_CALL OKeySet::absolute( sal_Int32 row ) throw(SQLException, 
Runtime
 {
     return absolute_checked(row,sal_True);
 }
-sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
+sal_Bool OKeySet::absolute_checked( sal_Int32 row, sal_Bool /* i_bFetchRow */ )
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::absolute" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
@@ -1301,10 +1297,7 @@ sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
     }
     if ( !fetchedRow )
     {
-        if ( i_bFetchRow )
-            refreshRow();
-        else
-            invalidateRow();
+        invalidateRow();
     }
 
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
@@ -1315,23 +1308,20 @@ sal_Bool SAL_CALL OKeySet::relative( sal_Int32 rows ) throw(SQLException, 
Runtim
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::relative" );
     if(!rows)
     {
-        refreshRow();
+        invalidateRow();
         return sal_True;
     }
     return absolute(getRow()+rows);
 }
 
-sal_Bool OKeySet::previous_checked( sal_Bool i_bFetchRow )
+sal_Bool OKeySet::previous_checked( sal_Bool /* i_bFetchRow */ )
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::previous" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     if(m_aKeyIter != m_aKeyMap.begin())
     {
         --m_aKeyIter;
-        if ( i_bFetchRow )
-            refreshRow();
-        else
-            invalidateRow();
+        invalidateRow();
     }
     return m_aKeyIter != m_aKeyMap.begin();
 }
@@ -1510,140 +1500,153 @@ bool OKeySet::fillAllRows()
 sal_Bool SAL_CALL OKeySet::wasNull(  ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::wasNull" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    if ( ! m_xRow.is() )
+        throwGenericSQLException(::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("Must call getFOO() 
for some FOO before wasNull()")), *this);
+
+    OSL_ENSURE(m_xRow.is(),"m_xRow is null! I've thrown, but function execution continued?");
     return m_xRow->wasNull();
 }
 
+inline void OKeySet::ensureRowForData( ) throw(SQLException, RuntimeException)
+{
+    if (! m_xRow.is() )
+        refreshRow();
+    if (! m_xRow.is() )
+        throwSQLException("Failed to refetch row", "02000", *this, -2);
+
+    OSL_ENSURE(m_xRow.is(),"m_xRow is null! I've called throwSQLException but execution 
continued?");
+}
+
 ::rtl::OUString SAL_CALL OKeySet::getString( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getString" 
);
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getString(columnIndex);
 }
 
 sal_Bool SAL_CALL OKeySet::getBoolean( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getBoolean" 
);
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBoolean(columnIndex);
 }
 
 sal_Int8 SAL_CALL OKeySet::getByte( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getByte" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getByte(columnIndex);
 }
 
 sal_Int16 SAL_CALL OKeySet::getShort( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getShort" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getShort(columnIndex);
 }
 
 sal_Int32 SAL_CALL OKeySet::getInt( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getInt" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getInt(columnIndex);
 }
 
 sal_Int64 SAL_CALL OKeySet::getLong( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getLong" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getLong(columnIndex);
 }
 
 float SAL_CALL OKeySet::getFloat( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getFloat" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getFloat(columnIndex);
 }
 
 double SAL_CALL OKeySet::getDouble( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getDouble" 
);
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getDouble(columnIndex);
 }
 
 Sequence< sal_Int8 > SAL_CALL OKeySet::getBytes( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getBytes" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBytes(columnIndex);
 }
 
 ::com::sun::star::util::Date SAL_CALL OKeySet::getDate( sal_Int32 columnIndex ) 
throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getDate" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getDate(columnIndex);
 }
 
 ::com::sun::star::util::Time SAL_CALL OKeySet::getTime( sal_Int32 columnIndex ) 
throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getTime" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getTime(columnIndex);
 }
 
 ::com::sun::star::util::DateTime SAL_CALL OKeySet::getTimestamp( sal_Int32 columnIndex ) 
throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", 
"OKeySet::getTimestamp" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getTimestamp(columnIndex);
 }
 
 Reference< ::com::sun::star::io::XInputStream > SAL_CALL OKeySet::getBinaryStream( sal_Int32 
columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", 
"OKeySet::getBinaryStream" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBinaryStream(columnIndex);
 }
 
 Reference< ::com::sun::star::io::XInputStream > SAL_CALL OKeySet::getCharacterStream( sal_Int32 
columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", 
"OKeySet::getCharacterStream" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getCharacterStream(columnIndex);
 }
 
 Any SAL_CALL OKeySet::getObject( sal_Int32 columnIndex, const Reference< 
::com::sun::star::container::XNameAccess >& typeMap ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getObject" 
);
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getObject(columnIndex,typeMap);
 }
 
 Reference< XRef > SAL_CALL OKeySet::getRef( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getRef" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getRef(columnIndex);
 }
 
 Reference< XBlob > SAL_CALL OKeySet::getBlob( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getBlob" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBlob(columnIndex);
 }
 
 Reference< XClob > SAL_CALL OKeySet::getClob( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getClob" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getClob(columnIndex);
 }
 
 Reference< XArray > SAL_CALL OKeySet::getArray( sal_Int32 columnIndex ) throw(SQLException, 
RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "Ocke.Janssen@sun.com", "OKeySet::getArray" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getArray(columnIndex);
 }
 
diff --git a/dbaccess/source/core/api/KeySet.hxx b/dbaccess/source/core/api/KeySet.hxx
index b98086d..35b2b9b 100644
--- a/dbaccess/source/core/api/KeySet.hxx
+++ b/dbaccess/source/core/api/KeySet.hxx
@@ -210,6 +210,7 @@ namespace dbaccess
         virtual sal_Bool SAL_CALL absolute( sal_Int32 row ) 
throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
         virtual sal_Bool SAL_CALL relative( sal_Int32 rows ) 
throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
         virtual sal_Bool SAL_CALL previous(  ) throw(::com::sun::star::sdbc::SQLException, 
::com::sun::star::uno::RuntimeException);
+        virtual void SAL_CALL ensureRowForData(  ) throw(::com::sun::star::sdbc::SQLException, 
::com::sun::star::uno::RuntimeException);
         virtual void SAL_CALL refreshRow(  ) throw(::com::sun::star::sdbc::SQLException, 
::com::sun::star::uno::RuntimeException);
         // ::com::sun::star::sdbcx::XRowLocate
         virtual ::com::sun::star::uno::Any SAL_CALL getBookmark() 
throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
-- 
1.7.10