Commit e7681c70 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Add location to SQL ScopedBlockingCall

Lift the location into the InitScopedBlockingCall to allow
debugging slow cases with slow-reports.

Bug: 1027929
Change-Id: Ife752fab110d11fac07a0646e0bc2353e9396ff1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003213Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732638}
parent 37970334
...@@ -217,7 +217,7 @@ void Database::StatementRef::Close(bool forced) { ...@@ -217,7 +217,7 @@ void Database::StatementRef::Close(bool forced) {
// TODO(paivanof@gmail.com): This should move to the beginning // TODO(paivanof@gmail.com): This should move to the beginning
// of the function. http://crbug.com/136655. // of the function. http://crbug.com/136655.
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
sqlite3_finalize(stmt_); sqlite3_finalize(stmt_);
stmt_ = nullptr; stmt_ = nullptr;
} }
...@@ -309,9 +309,9 @@ void Database::CloseInternal(bool forced) { ...@@ -309,9 +309,9 @@ void Database::CloseInternal(bool forced) {
// TODO(paivanof@gmail.com): This should move to the beginning // TODO(paivanof@gmail.com): This should move to the beginning
// of the function. http://crbug.com/136655. // of the function. http://crbug.com/136655.
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
// Reseting acquires a lock to ensure no dump is happening on the database // Resetting 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 // 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 // since the db is reset. memory_dump_provider_ could be null if db_ was
// poisoned. // poisoned.
...@@ -353,7 +353,7 @@ void Database::Preload() { ...@@ -353,7 +353,7 @@ void Database::Preload() {
} }
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
// Maximum number of bytes that will be prefetched from the database. // Maximum number of bytes that will be prefetched from the database.
// //
...@@ -636,7 +636,7 @@ bool Database::SetMmapAltStatus(int64_t status) { ...@@ -636,7 +636,7 @@ bool Database::SetMmapAltStatus(int64_t status) {
size_t Database::GetAppropriateMmapSize() { size_t Database::GetAppropriateMmapSize() {
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
// How much to map if no errors are found. 50MB encompasses the 99th // How much to map if no errors are found. 50MB encompasses the 99th
// percentile of Chrome databases in the wild, so this should be good. // percentile of Chrome databases in the wild, so this should be good.
...@@ -777,7 +777,7 @@ void Database::TrimMemory() { ...@@ -777,7 +777,7 @@ void Database::TrimMemory() {
// size, then backup that database over the existing database. // size, then backup that database over the existing database.
bool Database::Raze() { bool Database::Raze() {
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
if (!db_) { if (!db_) {
DCHECK(poisoned_) << "Cannot raze null db"; DCHECK(poisoned_) << "Cannot raze null db";
...@@ -1079,7 +1079,7 @@ int Database::ExecuteAndReturnErrorCode(const char* sql) { ...@@ -1079,7 +1079,7 @@ int Database::ExecuteAndReturnErrorCode(const char* sql) {
} }
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
int rc = SQLITE_OK; int rc = SQLITE_OK;
while ((rc == SQLITE_OK) && *sql) { while ((rc == SQLITE_OK) && *sql) {
...@@ -1198,7 +1198,7 @@ scoped_refptr<Database::StatementRef> Database::GetStatementImpl( ...@@ -1198,7 +1198,7 @@ scoped_refptr<Database::StatementRef> Database::GetStatementImpl(
return base::MakeRefCounted<StatementRef>(nullptr, nullptr, poisoned_); return base::MakeRefCounted<StatementRef>(nullptr, nullptr, poisoned_);
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
// TODO(pwnall): Cached statements (but not unique statements) should be // TODO(pwnall): Cached statements (but not unique statements) should be
// prepared with prepFlags set to SQLITE_PREPARE_PERSISTENT. // prepared with prepFlags set to SQLITE_PREPARE_PERSISTENT.
...@@ -1247,7 +1247,7 @@ std::string Database::GetSchema() const { ...@@ -1247,7 +1247,7 @@ std::string Database::GetSchema() const {
bool Database::IsSQLValid(const char* sql) { bool Database::IsSQLValid(const char* sql) {
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
if (!db_) { if (!db_) {
DCHECK(poisoned_) << "Illegal use of Database without a db"; DCHECK(poisoned_) << "Illegal use of Database without a db";
return false; return false;
...@@ -1352,7 +1352,7 @@ bool Database::OpenInternal(const std::string& file_name, ...@@ -1352,7 +1352,7 @@ bool Database::OpenInternal(const std::string& file_name,
} }
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(&scoped_blocking_call); InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
EnsureSqliteInitialized(); EnsureSqliteInitialized();
......
...@@ -129,7 +129,7 @@ class COMPONENT_EXPORT(SQL) Database { ...@@ -129,7 +129,7 @@ class COMPONENT_EXPORT(SQL) Database {
// histogram is recorded. // histogram is recorded.
void AddTaggedHistogram(const std::string& name, int sample) const; void AddTaggedHistogram(const std::string& name, int sample) const;
// Track various API calls and results. Values corrospond to UMA // Track various API calls and results. Values correspond to UMA
// histograms, do not modify, or add or delete other than directly // histograms, do not modify, or add or delete other than directly
// before EVENT_MAX_VALUE. // before EVENT_MAX_VALUE.
enum Events { enum Events {
...@@ -520,11 +520,13 @@ class COMPONENT_EXPORT(SQL) Database { ...@@ -520,11 +520,13 @@ class COMPONENT_EXPORT(SQL) Database {
void CloseInternal(bool forced); void CloseInternal(bool forced);
// Construct a ScopedBlockingCall to annotate IO calls, but only if // Construct a ScopedBlockingCall to annotate IO calls, but only if
// database wasn't open in memory. // database wasn't open in memory. ScopedBlockingCall uses |from_here| to
// declare its blocking execution scope (see https://www.crbug/934302).
void InitScopedBlockingCall( void InitScopedBlockingCall(
const base::Location& from_here,
base::Optional<base::ScopedBlockingCall>* scoped_blocking_call) const { base::Optional<base::ScopedBlockingCall>* scoped_blocking_call) const {
if (!in_memory_) if (!in_memory_)
scoped_blocking_call->emplace(FROM_HERE, base::BlockingType::MAY_BLOCK); scoped_blocking_call->emplace(from_here, base::BlockingType::MAY_BLOCK);
} }
// Internal helper for Does*Exist() functions. // Internal helper for Does*Exist() functions.
...@@ -588,11 +590,13 @@ class COMPONENT_EXPORT(SQL) Database { ...@@ -588,11 +590,13 @@ class COMPONENT_EXPORT(SQL) Database {
void Close(bool forced); void Close(bool forced);
// Construct a ScopedBlockingCall to annotate IO calls, but only if // Construct a ScopedBlockingCall to annotate IO calls, but only if
// database wasn't open in memory. // database wasn't open in memory. ScopedBlockingCall uses |from_here| to
// declare its blocking execution scope (see https://www.crbug/934302).
void InitScopedBlockingCall( void InitScopedBlockingCall(
const base::Location& from_here,
base::Optional<base::ScopedBlockingCall>* scoped_blocking_call) const { base::Optional<base::ScopedBlockingCall>* scoped_blocking_call) const {
if (database_) if (database_)
database_->InitScopedBlockingCall(scoped_blocking_call); database_->InitScopedBlockingCall(from_here, scoped_blocking_call);
} }
private: private:
......
...@@ -58,7 +58,7 @@ int Statement::StepInternal() { ...@@ -58,7 +58,7 @@ int Statement::StepInternal() {
return SQLITE_ERROR; return SQLITE_ERROR;
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
ref_->InitScopedBlockingCall(&scoped_blocking_call); ref_->InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
stepped_ = true; stepped_ = true;
int ret = sqlite3_step(ref_->stmt()); int ret = sqlite3_step(ref_->stmt());
...@@ -76,7 +76,7 @@ bool Statement::Step() { ...@@ -76,7 +76,7 @@ bool Statement::Step() {
void Statement::Reset(bool clear_bound_vars) { void Statement::Reset(bool clear_bound_vars) {
base::Optional<base::ScopedBlockingCall> scoped_blocking_call; base::Optional<base::ScopedBlockingCall> scoped_blocking_call;
ref_->InitScopedBlockingCall(&scoped_blocking_call); ref_->InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
if (is_valid()) { if (is_valid()) {
if (clear_bound_vars) if (clear_bound_vars)
sqlite3_clear_bindings(ref_->stmt()); sqlite3_clear_bindings(ref_->stmt());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment