[tracing] Add separate dump provider for sql connection

sql/connection.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sql/connection.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <string.h>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/debug/dump_without_crashing.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/format_macros.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "base/trace_event/memory_dump_manager.h"
-#include "base/trace_event/process_memory_dump.h"
+#include "sql/connection_memory_dump_provider.h"
 #include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "third_party/sqlite/sqlite3.h"
 
 #if defined(OS_IOS) && defined(USE_SYSTEM_SQLITE)
 #include "third_party/sqlite/src/ext/icu/sqliteicu.h"
 #endif
 
 namespace {
 
@@ skipping 206 matching lines @@
   int basic_error = error & 0xff;
 
   // These errors relate more to the runtime context of the system than to
   // errors with a SQL statement or with the schema, so they aren't generally
   // interesting to flag.  This list is not comprehensive.
   return basic_error == SQLITE_BUSY ||
       basic_error == SQLITE_NOTADB ||
       basic_error == SQLITE_CORRUPT;
 }
 
-bool Connection::OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
-                              base::trace_event::ProcessMemoryDump* pmd) {
-  if (args.level_of_detail ==
-          base::trace_event::MemoryDumpLevelOfDetail::LIGHT ||
-      !db_) {
-    return true;
-  }
-
-  // The high water mark is not tracked for the following usages.
-  int cache_size, dummy_int;
-  sqlite3_db_status(db_, SQLITE_DBSTATUS_CACHE_USED, &cache_size, &dummy_int,
-                    0 /* resetFlag */);
-  int schema_size;
-  sqlite3_db_status(db_, SQLITE_DBSTATUS_SCHEMA_USED, &schema_size, &dummy_int,
-                    0 /* resetFlag */);
-  int statement_size;
-  sqlite3_db_status(db_, SQLITE_DBSTATUS_STMT_USED, &statement_size, &dummy_int,
-                    0 /* resetFlag */);
-
-  std::string name = base::StringPrintf(
-      "sqlite/%s_connection/%p",
-      histogram_tag_.empty() ? "Unknown" : histogram_tag_.c_str(), this);
-  base::trace_event::MemoryAllocatorDump* dump = pmd->CreateAllocatorDump(name);
-  dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
-                  base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                  cache_size + schema_size + statement_size);
-  dump->AddScalar("cache_size",
-                  base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                  cache_size);
-  dump->AddScalar("schema_size",
-                  base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                  schema_size);
-  dump->AddScalar("statement_size",
-                  base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                  statement_size);
-  return true;
-}
-
 void Connection::ReportDiagnosticInfo(int extended_error, Statement* stmt) {
   AssertIOAllowed();
 
   std::string debug_info;
   const int error = (extended_error & 0xFF);
   if (error == SQLITE_CORRUPT) {
     debug_info = CollectCorruptionInfo();
   } else {
     debug_info = CollectErrorInfo(extended_error, stmt);
   }
@@ skipping 74 matching lines @@
       poisoned_(false),
       mmap_disabled_(false),
       mmap_enabled_(false),
       total_changes_at_last_release_(0),
       stats_histogram_(NULL),
       commit_time_histogram_(NULL),
       autocommit_time_histogram_(NULL),
       update_time_histogram_(NULL),
       query_time_histogram_(NULL),
       clock_(new TimeSource()) {
-  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
-      this, "sql::Connection", nullptr);
 }
 
 Connection::~Connection() {
-  base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
-      this);
   Close();
 }
 
 void Connection::RecordEvent(Events event, size_t count) {
   for (size_t i = 0; i < count; ++i) {
     UMA_HISTOGRAM_ENUMERATION("Sqlite.Stats", event, EVENT_MAX_VALUE);
   }
 
   if (stats_histogram_) {
     for (size_t i = 0; i < count; ++i) {
@@ skipping 98 matching lines @@
 
   if (db_) {
     // Call to AssertIOAllowed() cannot go at the beginning of the function
     // because Close() must be called from destructor to clean
     // statement_cache_, it won't cause any disk access and it most probably
     // will happen on thread not allowing disk access.
     // TODO(paivanof@gmail.com): This should move to the beginning
     // of the function. http://crbug.com/136655.
     AssertIOAllowed();
 
+    // Reseting acquires a lock to ensure no dump is happening on the database
+    // at the same time. Unregister takes ownership of provider and it is safe
+    // since the db is reset. memory_dump_provider_ could be null if db_ was
+    // poisoned.
+    if (memory_dump_provider_) {
+      memory_dump_provider_->ResetDatabase();
+      base::trace_event::MemoryDumpManager::GetInstance()
+          ->UnregisterAndDeleteDumpProviderSoon(
+              std::move(memory_dump_provider_));
+    }

[Scott Hess - ex-Googler, 2016/01/12 19:33:14]

OK, reasoning this out, a dump before the reset is fine, a dump after the reset
if fine (db_ will be null), and the reset is locked to push a dump during the
reset to after.  Then ownership is removed from here, and passed over to the
manager, so it that unregister/delete will be interleaved with dumps on that
thread, so the necessary lock will exist until it is no longer needed.

I think that's it.

[Primiano Tucci (use gerrit), 2016/01/12 19:38:02]

Yup, this reasoning matches perfectly ours :)

+
     int rc = sqlite3_close(db_);
     if (rc != SQLITE_OK) {
       UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.CloseFailure", rc);
       DLOG(FATAL) << "sqlite3_close failed: " << GetErrorMessage();
     }
   }
   db_ = NULL;
 }
 
 void Connection::Close() {
@@ skipping 1317 matching lines @@
 
   // Determine if memory-mapping has actually been enabled.  The Execute() above
   // can succeed without changing the amount mapped.
   mmap_enabled_ = false;
   {
     Statement s(GetUniqueStatement("PRAGMA mmap_size"));
     if (s.Step() && s.ColumnInt64(0) > 0)
       mmap_enabled_ = true;
   }
 
+  DCHECK(!memory_dump_provider_);
+  memory_dump_provider_.reset(
+      new ConnectionMemoryDumpProvider(db_, histogram_tag_));
+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      memory_dump_provider_.get(), "sql::Connection", nullptr);
+
   return true;
 }
 
 void Connection::DoRollback() {
   Statement rollback(GetCachedStatement(SQL_FROM_HERE, "ROLLBACK"));
 
   // Collect the rollback time manually, sql::Statement would register it as
   // query time only.
   const base::TimeTicks before = Now();
   rollback.RunWithoutTimers();
@@ skipping 123 matching lines @@
   ignore_result(Execute(kNoWritableSchema));
 
   return ret;
 }
 
 base::TimeTicks TimeSource::Now() {
   return base::TimeTicks::Now();
 }
 
 }  // namespace sql