From d55ee14c3e752008080dac0541eb7ea7e4e88ae6 Mon Sep 17 00:00:00 2001 From: Ben Turner Date: Tue, 19 May 2015 09:17:10 -0700 Subject: [PATCH] Bug 1162176, Part 1. r=mak. --- db/sqlite3/src/sqlite.def | 1 + storage/src/mozStorageConnection.cpp | 56 ++++++++--- storage/src/mozStorageConnection.h | 8 +- toolkit/components/telemetry/Telemetry.cpp | 107 ++++++++++++++------- 4 files changed, 121 insertions(+), 51 deletions(-) diff --git a/db/sqlite3/src/sqlite.def b/db/sqlite3/src/sqlite.def index c9f741a5796..09ec0a831f4 100644 --- a/db/sqlite3/src/sqlite.def +++ b/db/sqlite3/src/sqlite.def @@ -51,6 +51,7 @@ EXPORTS sqlite3_create_function16 sqlite3_create_module sqlite3_data_count + sqlite3_db_filename sqlite3_db_handle sqlite3_db_mutex sqlite3_db_status diff --git a/storage/src/mozStorageConnection.cpp b/storage/src/mozStorageConnection.cpp index 89f4d05d6f6..15f89baf4f2 100644 --- a/storage/src/mozStorageConnection.cpp +++ b/storage/src/mozStorageConnection.cpp @@ -590,7 +590,13 @@ Connection::initialize() return convertResultCode(srv); } - return initializeInternal(nullptr); + // Do not set mDatabaseFile or mFileURL here since this is a "memory" + // database. + + nsresult rv = initializeInternal(); + NS_ENSURE_SUCCESS(rv, rv); + + return NS_OK; } nsresult @@ -614,11 +620,13 @@ Connection::initialize(nsIFile *aDatabaseFile) return convertResultCode(srv); } - rv = initializeInternal(aDatabaseFile); - NS_ENSURE_SUCCESS(rv, rv); - + // Do not set mFileURL here since this is database does not have an associated + // URL. mDatabaseFile = aDatabaseFile; + rv = initializeInternal(); + NS_ENSURE_SUCCESS(rv, rv); + return NS_OK; } @@ -644,19 +652,40 @@ Connection::initialize(nsIFileURL *aFileURL) return convertResultCode(srv); } - rv = initializeInternal(databaseFile); - NS_ENSURE_SUCCESS(rv, rv); - + // Set both mDatabaseFile and mFileURL here. mFileURL = aFileURL; mDatabaseFile = databaseFile; + rv = initializeInternal(); + NS_ENSURE_SUCCESS(rv, rv); + return NS_OK; } - nsresult -Connection::initializeInternal(nsIFile* aDatabaseFile) +Connection::initializeInternal() { + MOZ_ASSERT(mDBConn); + + if (mFileURL) { + const char* dbPath = ::sqlite3_db_filename(mDBConn, "main"); + MOZ_ASSERT(dbPath); + + const char* telemetryFilename = + ::sqlite3_uri_parameter(dbPath, "telemetryFilename"); + if (telemetryFilename) { + if (NS_WARN_IF(*telemetryFilename == '\0')) { + return NS_ERROR_INVALID_ARG; + } + mTelemetryFilename = telemetryFilename; + } + } + + if (mTelemetryFilename.IsEmpty()) { + mTelemetryFilename = getFilename(); + MOZ_ASSERT(!mTelemetryFilename.IsEmpty()); + } + // Properly wrap the database handle's mutex. sharedDBMutex.initWithMutex(sqlite3_db_mutex(mDBConn)); @@ -668,11 +697,8 @@ Connection::initializeInternal(nsIFile* aDatabaseFile) if (PR_LOG_TEST(gStorageLog, PR_LOG_DEBUG)) { ::sqlite3_trace(mDBConn, tracefunc, this); - nsAutoCString leafName(":memory"); - if (aDatabaseFile) - (void)aDatabaseFile->GetNativeLeafName(leafName); PR_LOG(gStorageLog, PR_LOG_NOTICE, ("Opening connection to '%s' (%p)", - leafName.get(), this)); + mTelemetryFilename.get(), this)); } int64_t pageSize = Service::getDefaultPageSize(); @@ -1025,7 +1051,7 @@ Connection::stepStatement(sqlite3 *aNativeConnection, sqlite3_stmt *aStatement) : Telemetry::kSlowSQLThresholdForHelperThreads; if (duration.ToMilliseconds() >= threshold) { nsDependentCString statementString(::sqlite3_sql(aStatement)); - Telemetry::RecordSlowSQLStatement(statementString, getFilename(), + Telemetry::RecordSlowSQLStatement(statementString, mTelemetryFilename, duration.ToMilliseconds()); } @@ -1111,7 +1137,7 @@ Connection::executeSql(sqlite3 *aNativeConnection, const char *aSqlString) : Telemetry::kSlowSQLThresholdForHelperThreads; if (duration.ToMilliseconds() >= threshold) { nsDependentCString statementString(aSqlString); - Telemetry::RecordSlowSQLStatement(statementString, getFilename(), + Telemetry::RecordSlowSQLStatement(statementString, mTelemetryFilename, duration.ToMilliseconds()); } diff --git a/storage/src/mozStorageConnection.h b/storage/src/mozStorageConnection.h index ae1611aef74..dd292258952 100644 --- a/storage/src/mozStorageConnection.h +++ b/storage/src/mozStorageConnection.h @@ -230,7 +230,7 @@ public: private: ~Connection(); - nsresult initializeInternal(nsIFile *aDatabaseFile); + nsresult initializeInternal(); /** * Sets the database into a closed state so no further actions can be @@ -288,6 +288,12 @@ private: nsCOMPtr mFileURL; nsCOMPtr mDatabaseFile; + /** + * The filename that will be reported to telemetry for this connection. By + * default this will be the leaf of the path to the database file. + */ + nsCString mTelemetryFilename; + /** * Lazily created thread for asynchronous statement execution. Consumers * should use getAsyncExecutionTarget rather than directly accessing this diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index c734f6c48f2..ba4aabe3d6f 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -736,9 +736,6 @@ private: static TelemetryImpl *sTelemetry; AutoHashtable mPrivateSQL; AutoHashtable mSanitizedSQL; - // This gets marked immutable in debug builds, so we can't use - // AutoHashtable here. - nsTHashtable mTrackedDBs; Mutex mHashMutex; HangReports mHangReports; Mutex mHangReportsMutex; @@ -1862,35 +1859,6 @@ mCachedTelemetryData(false), mLastShutdownTime(0), mFailedLockCount(0) { - // A whitelist to prevent Telemetry reporting on Addon & Thunderbird DBs - const char *trackedDBs[] = { - "818200132aebmoouht.sqlite", // IndexedDB for about:home, see aboutHome.js - "addons.sqlite", - "content-prefs.sqlite", - "cookies.sqlite", - "downloads.sqlite", - "extensions.sqlite", - "formhistory.sqlite", - "healthreport.sqlite", - "index.sqlite", - "netpredictions.sqlite", - "permissions.sqlite", - "places.sqlite", - "reading-list.sqlite", - "search.sqlite", - "signons.sqlite", - "urlclassifier3.sqlite", - "webappsstore.sqlite" - }; - - for (size_t i = 0; i < ArrayLength(trackedDBs); i++) - mTrackedDBs.PutEntry(nsDependentCString(trackedDBs[i])); - -#ifdef DEBUG - // Mark immutable to prevent asserts on simultaneous access from multiple threads - mTrackedDBs.MarkImmutable(); -#endif - // Populate the static histogram name->id cache. // Note that the histogram names are statically allocated. for (uint32_t i = 0; i < Telemetry::HistogramCount; i++) { @@ -3426,6 +3394,51 @@ TelemetryImpl::SanitizeSQL(const nsACString &sql) { return output; } +// A whitelist mechanism to prevent Telemetry reporting on Addon & Thunderbird +// DBs. +struct TrackedDBEntry +{ + const char* mName; + const uint32_t mNameLength; + + // This struct isn't meant to be used beyond the static arrays below. + TrackedDBEntry() = delete; + TrackedDBEntry(TrackedDBEntry&) = delete; +}; + +#define TRACKEDDB_ENTRY(_name) { _name, (sizeof(_name) - 1) } + +// A whitelist of database names. If the database name exactly matches one of +// these then its SQL statements will always be recorded. +static MOZ_CONSTEXPR_VAR TrackedDBEntry kTrackedDBs[] = { + // IndexedDB for about:home, see aboutHome.js + TRACKEDDB_ENTRY("818200132aebmoouht.sqlite"), + TRACKEDDB_ENTRY("addons.sqlite"), + TRACKEDDB_ENTRY("content-prefs.sqlite"), + TRACKEDDB_ENTRY("cookies.sqlite"), + TRACKEDDB_ENTRY("downloads.sqlite"), + TRACKEDDB_ENTRY("extensions.sqlite"), + TRACKEDDB_ENTRY("formhistory.sqlite"), + TRACKEDDB_ENTRY("healthreport.sqlite"), + TRACKEDDB_ENTRY("index.sqlite"), + TRACKEDDB_ENTRY("netpredictions.sqlite"), + TRACKEDDB_ENTRY("permissions.sqlite"), + TRACKEDDB_ENTRY("places.sqlite"), + TRACKEDDB_ENTRY("reading-list.sqlite"), + TRACKEDDB_ENTRY("search.sqlite"), + TRACKEDDB_ENTRY("signons.sqlite"), + TRACKEDDB_ENTRY("urlclassifier3.sqlite"), + TRACKEDDB_ENTRY("webappsstore.sqlite") +}; + +// A whitelist of database name prefixes. If the database name begins with +// one of these prefixes then its SQL statements will always be recorded. +static const TrackedDBEntry kTrackedDBPrefixes[] = { + TRACKEDDB_ENTRY("indexedDB-") +}; + +#undef TRACKEDDB_ENTRY + // Slow SQL statements will be automatically // trimmed to kMaxSlowStatementLength characters. // This limit doesn't include the ellipsis and DB name, @@ -3437,11 +3450,36 @@ TelemetryImpl::RecordSlowStatement(const nsACString &sql, const nsACString &dbName, uint32_t delay) { + MOZ_ASSERT(!sql.IsEmpty()); + MOZ_ASSERT(!dbName.IsEmpty()); + if (!sTelemetry || !sTelemetry->mCanRecordExtended) return; - bool isFirefoxDB = sTelemetry->mTrackedDBs.Contains(dbName); - if (isFirefoxDB) { + bool recordStatement = false; + + for (const TrackedDBEntry& nameEntry : kTrackedDBs) { + MOZ_ASSERT(nameEntry.mNameLength); + const nsDependentCString name(nameEntry.mName, nameEntry.mNameLength); + if (dbName == name) { + recordStatement = true; + break; + } + } + + if (!recordStatement) { + for (const TrackedDBEntry& prefixEntry : kTrackedDBPrefixes) { + MOZ_ASSERT(prefixEntry.mNameLength); + const nsDependentCString prefix(prefixEntry.mName, + prefixEntry.mNameLength); + if (StringBeginsWith(dbName, prefix)) { + recordStatement = true; + break; + } + } + } + + if (recordStatement) { nsAutoCString sanitizedSQL(SanitizeSQL(sql)); if (sanitizedSQL.Length() > kMaxSlowStatementLength) { sanitizedSQL.SetLength(kMaxSlowStatementLength); @@ -3571,7 +3609,6 @@ TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) n += mPrivateSQL.SizeOfExcludingThis(aMallocSizeOf); n += mSanitizedSQL.SizeOfExcludingThis(aMallocSizeOf); } - n += mTrackedDBs.SizeOfExcludingThis(aMallocSizeOf); { // Scope for mHangReportsMutex lock MutexAutoLock lock(mHangReportsMutex); n += mHangReports.SizeOfExcludingThis(aMallocSizeOf);