Bug 481261 - Expose when queries are being inefficient in debug builds.
authorShawn Wilsher <sdwilsh@shawnwilsher.com>
Thu, 12 Mar 2009 14:30:15 -0400
changeset 26107 6c56947c095f551e776d3ef9ca27d81f4c7f6042
parent 26106 bf3a15860df4ab26e6b51ea51d8265f70960f96d
child 26108 7252af11301d40c628bc3a09cdae031bca224bfc
push id5905
push usersdwilsh@shawnwilsher.com
push dateThu, 12 Mar 2009 18:32:32 +0000
treeherdermozilla-central@6c56947c095f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs481261
milestone1.9.2a1pre
Bug 481261 - Expose when queries are being inefficient in debug builds. This adds a warning in debug builds about queries that do not use an index to sort, which tend to be much much slower. r=asuth
db/sqlite3/src/sqlite.def
storage/src/Makefile.in
storage/src/mozStorageEvents.cpp
storage/src/mozStoragePrivateHelpers.cpp
storage/src/mozStoragePrivateHelpers.h
storage/src/mozStorageStatement.cpp
--- a/db/sqlite3/src/sqlite.def
+++ b/db/sqlite3/src/sqlite.def
@@ -146,16 +146,17 @@ EXPORTS
         sqlite3_rollback_hook
         sqlite3_set_authorizer
         sqlite3_set_auxdata
         sqlite3_shutdown
         sqlite3_sleep
         sqlite3_snprintf
         sqlite3_sql
         sqlite3_step
+        sqlite3_stmt_status
         sqlite3_thread_cleanup
         sqlite3_total_changes
         sqlite3_trace
         sqlite3_transfer_bindings
         sqlite3_update_hook
         sqlite3_user_data
         sqlite3_value_blob
         sqlite3_value_bytes
--- a/storage/src/Makefile.in
+++ b/storage/src/Makefile.in
@@ -70,16 +70,17 @@ CPPSRCS = \
   mozStorageStatementRow.cpp \
   mozStorageValueArray.cpp \
   mozStorageUnicodeFunctions.cpp \
   mozStorageRow.cpp \
   mozStorageResultSet.cpp \
   mozStorageError.cpp \
   mozStorageEvents.cpp \
   mozStorageStatementJSHelper.cpp \
+  mozStoragePrivateHelpers.cpp \
   $(NULL)
 
 LOCAL_INCLUDES = \
 	$(SQLITE_CFLAGS)
 
 # This is the default value.  If we ever change it when compiling sqlite, we
 # will need to change it here as well.
 DEFINES += -DSQLITE_MAX_LIKE_PATTERN_LENGTH=50000
--- a/storage/src/mozStorageEvents.cpp
+++ b/storage/src/mozStorageEvents.cpp
@@ -47,16 +47,17 @@
 
 #include "mozIStorageStatementCallback.h"
 #include "mozIStoragePendingStatement.h"
 #include "mozStorageHelper.h"
 #include "mozStorageResultSet.h"
 #include "mozStorageRow.h"
 #include "mozStorageConnection.h"
 #include "mozStorageError.h"
+#include "mozStoragePrivateHelpers.h"
 #include "mozStorageEvents.h"
 
 /**
  * The following constants help batch rows into result sets.
  * MAX_MILLISECONDS_BETWEEN_RESULTS was chosen because any user-based task that
  * takes less than 200 milliseconds is considered to feel instantaneous to end
  * users.  MAX_ROWS_PER_RESULT was arbitrarily chosen to reduce the number of
  * dispatches to calling thread, while also providing reasonably-sized sets of
@@ -421,16 +422,21 @@ private:
       // Drop our mutex - NotifyError doesn't want it held
       mutex.unlock();
 
       // Notify, and stop processing statements.
       (void)NotifyError(mozIStorageError::ERROR, "");
       return PR_FALSE;
     }
 
+#ifdef DEBUG
+    // Check to make sure that this statement was smart about what it did.
+    CheckAndLogStatementPerformance(aStatement);
+#endif
+
     // If we are done, we need to set our state accordingly while we still
     // hold our lock.  We would have already returned if we were canceled or had
     // an error at this point.
     if (aFinished)
       mState = COMPLETED;
 
     return PR_TRUE;
   }
new file mode 100644
--- /dev/null
+++ b/storage/src/mozStoragePrivateHelpers.cpp
@@ -0,0 +1,106 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * vim: sw=2 ts=2 sts=2 et
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Vladimir Vukicevic <vladimir.vukicevic@oracle.com>
+ *   Shawn Wilsher <me@shawnwilsher.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "sqlite3.h"
+
+#include "nsString.h"
+#include "nsError.h"
+
+#include "mozStoragePrivateHelpers.h"
+
+nsresult
+ConvertResultCode(int aSQLiteResultCode)
+{
+  switch (aSQLiteResultCode) {
+    case SQLITE_OK:
+    case SQLITE_ROW:
+    case SQLITE_DONE:
+      return NS_OK;
+    case SQLITE_CORRUPT:
+    case SQLITE_NOTADB:
+      return NS_ERROR_FILE_CORRUPTED;
+    case SQLITE_PERM:
+    case SQLITE_CANTOPEN:
+      return NS_ERROR_FILE_ACCESS_DENIED;
+    case SQLITE_BUSY:
+      return NS_ERROR_STORAGE_BUSY;
+    case SQLITE_LOCKED:
+      return NS_ERROR_FILE_IS_LOCKED;
+    case SQLITE_READONLY:
+      return NS_ERROR_FILE_READ_ONLY;
+    case SQLITE_IOERR:
+      return NS_ERROR_STORAGE_IOERR;
+    case SQLITE_FULL:
+    case SQLITE_TOOBIG:
+      return NS_ERROR_FILE_NO_DEVICE_SPACE;
+    case SQLITE_NOMEM:
+      return NS_ERROR_OUT_OF_MEMORY;
+    case SQLITE_MISUSE:
+      return NS_ERROR_UNEXPECTED;
+    case SQLITE_ABORT:
+    case SQLITE_INTERRUPT:
+      return NS_ERROR_ABORT;
+  }
+
+  // generic error
+  return NS_ERROR_FAILURE;
+}
+
+void
+CheckAndLogStatementPerformance(sqlite3_stmt *aStatement)
+{
+  // Check to see if the query performed sorting operations or not.  If it
+  // did, it may need to be optimized!
+  int count = sqlite3_stmt_status(aStatement, SQLITE_STMTSTATUS_SORT, 1);
+  if (count <= 0)
+    return;
+
+  nsCAutoString message;
+  message.AppendInt(count);
+  if (count == 1)
+    message.Append(" sort operation has ");
+  else
+    message.Append(" sort operations have ");
+  message.Append("occurred for the SQL statement '");
+  message.Append(sqlite3_sql(aStatement));
+  message.Append("'.  This may indicate an opportunity to improve performance "
+                 "through the careful use of indexes.");
+  NS_WARNING(message.get());
+}
--- a/storage/src/mozStoragePrivateHelpers.h
+++ b/storage/src/mozStoragePrivateHelpers.h
@@ -45,51 +45,27 @@
 
 /**
  * This file contains convenience methods for mozStorage.
  */
 
 /**
  * Converts a SQLite return code to an nsresult return code.
  *
- * @param srv The SQLite return code.
- * @return The corresponding nsresult code.
+ * @param aSQLiteResultCode
+ *        The SQLite return code to convert.
+ * @returns the corresponding nsresult code for aSQLiteResultCode.
  */
-static nsresult
-ConvertResultCode(int srv)
-{
-    switch (srv) {
-        case SQLITE_OK:
-        case SQLITE_ROW:
-        case SQLITE_DONE:
-            return NS_OK;
-        case SQLITE_CORRUPT:
-        case SQLITE_NOTADB:
-            return NS_ERROR_FILE_CORRUPTED;
-        case SQLITE_PERM:
-        case SQLITE_CANTOPEN:
-            return NS_ERROR_FILE_ACCESS_DENIED;
-        case SQLITE_BUSY:
-            return NS_ERROR_STORAGE_BUSY;
-        case SQLITE_LOCKED:
-            return NS_ERROR_FILE_IS_LOCKED;
-        case SQLITE_READONLY:
-            return NS_ERROR_FILE_READ_ONLY;
-        case SQLITE_IOERR:
-            return NS_ERROR_STORAGE_IOERR;
-        case SQLITE_FULL:
-        case SQLITE_TOOBIG:
-            return NS_ERROR_FILE_NO_DEVICE_SPACE;
-        case SQLITE_NOMEM:
-            return NS_ERROR_OUT_OF_MEMORY;
-        case SQLITE_MISUSE:
-            return NS_ERROR_UNEXPECTED;
-        case SQLITE_ABORT:
-        case SQLITE_INTERRUPT:
-            return NS_ERROR_ABORT;
-    }
+nsresult ConvertResultCode(int aSQLiteResultCode);
 
-    // generic error
-    return NS_ERROR_FAILURE;
-}
+/**
+ * Checks the performance of a SQLite statement and logs a warning with
+ * NS_WARNING.  Currently this only checks the number of sort operations done
+ * on a statement, and if more than zero have been done, the statement can be
+ * made faster with the careful use of an index.
+ *
+ * @param aStatement
+ *        The sqlite3_stmt object to check.
+ */
+void CheckAndLogStatementPerformance(sqlite3_stmt *aStatement);
 
 #endif // _MOZSTORAGEPRIVATEHELPERS_H_
 
--- a/storage/src/mozStorageStatement.cpp
+++ b/storage/src/mozStorageStatement.cpp
@@ -439,19 +439,23 @@ mozStorageStatement::GetColumnIndex(cons
 
 /* void reset (); */
 NS_IMETHODIMP
 mozStorageStatement::Reset()
 {
     if (!mDBConnection || !mDBStatement)
         return NS_ERROR_NOT_INITIALIZED;
 
+#ifdef DEBUG
     PR_LOG(gStorageLog, PR_LOG_DEBUG, ("Resetting statement: '%s'",
                                        sqlite3_sql(mDBStatement)));
 
+    CheckAndLogStatementPerformance(mDBStatement);
+#endif
+
     sqlite3_reset(mDBStatement);
     sqlite3_clear_bindings(mDBStatement);
 
     mExecuting = PR_FALSE;
 
     return NS_OK;
 }