This CL removes two classes of histograms.
1) SQL statement timing distributions, in the histograms below.
* Sqlite.AutoCommitTime.*
* Sqlite.CommitTime.*
* Sqlite.QueryTime.*
* Sqlite.UpdateTime.*
The timing distributions cover all SQL statements issued by each browser
feature built on top of SQLite, so they collect a large volume of data.
Their usefulness does not measure up to the amount of data collected,
for the reasons below.
* The histograms are too coarse to be useful as feature-level metrics.
The features whose owners are interested in optimizing performance
should deploy metrics that are specific to concrete investigations and
experiments.
* The histograms are less useful as SQLite-level metrics than the
heartbeat metrics that show overall Chrome performance.
2) Statistics about successful SQL statements and transactions, expressed
as the following enum values in Sqlite.Stats.*
* EVENT_STATEMENT_RUN
* EVENT_STATEMENT_ROWS
* EVENT_STATEMENT_SUCCESS
* EVENT_EXECUTE
* EVENT_CHANGES_AUTOCOMMIT
* EVENT_CHANGES
* EVENT_BEGIN
* EVENT_COMMIT
* EVENT_ROLLBACK
* EVENT_MMAP_SUCCESS_NEW
* EVENT_MMAP_SUCCESS_PARTIAL
* EVENT_MMAP_SUCCESS_NO_PROGRESS
These statistics cover all SQL statements / transactions used by each
browser features. Like the timing distributions above, they end up
collecting a lot of data, and aren't very useful. Raw SQL statement
counts are at best a very indirect measure of each feature's usage.
This CL removes the success outcomes, but keeps the error outcomes.
The metrics for errors are recorded much less often, as SQL operations
fail rarely, and are sufficient for computing error rates, which are a
bit more actionable.
Bug: 935824
Change-Id: Ia8f8c13a45ba78819bab8f130509cd6a8c6842a7
Reviewed-on: https://chromium-review.googlesource.com/c/1488952
Commit-Queue: Steven Holte <holte@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635668}
base::AssertBlockingAllowedDeprecated is deprecated in favor of
ScopedBlockingCall, which serves as a precise annotation of
the scope that may/will block.
Please make sure of the following:
- ScopedBlockingCall is instantiated in a scope with minimal CPU usage.
If this is not the case, ScopedBlockingCall should be instantiated
closer to the blocking call. See scoped_blocking_call.h for more
info. Please let me know when/where the blocking call happens if this needs
to be changed.
- Parameter |blocking_type| matches expectation:
MAY_BLOCK: The call might block (e.g. file I/O that might hit in memory cache).
WILL_BLOCK: The call will definitely block (e.g. cache already checked and now pinging
server synchronously).
See BlockingType for more info. While I assumed MAY_BLOCK by default, that might
not be the best fit if we know that this callsite is guaranteed to block.
- The ScopedBlockingCall's scope covers the entirety of the blocking operation
previously asserted against by the AssertBlockingAllowed().
- Calls to blocking //base APIs don't need to be annotated
with ScopedBlockingCall. All blocking //base APIs (e.g. base::ReadFileToString, base::File::Read,
base::SysInfo::AmountOfFreeDiskSpace, base::WaitableEvent::Wait, etc.) have their
own internal annotations.
Refer to the top-level CL if necessary :
https://chromium-review.googlesource.com/c/chromium/src/+/1338391
Please CQ if LGTY!
This CL was uploaded by git cl split.
R=cmumford@chromium.org
Bug: 903957
Change-Id: Ia7d71b3bc2536dad0e6adc669aebe36addfd7fd0
Reviewed-on: https://chromium-review.googlesource.com/c/1365805
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Cr-Commit-Position: refs/heads/master@{#629836}
sql::Connection represents an opened SQLite database. Pedantically, the
name is correct. At the same time, it is inconsistent with the naming
convention of similar software -- IndexedDB, LevelDB, and WebSQL all use
a derivative of "Database" (IDBDatabase, DB, WindowDatabase) to name
this concept. Furthermore, most sql::Connection instances have names
like "db" and "database".
This CL renames the class to sql::Database, reducing the cognitive load
caused by dissonances like "sql::Connection db_".
TBR=mpearson
Bug: none
Change-Id: I8abf34016bca236b6e38a772f5848e7ea9bf1c59
Reviewed-on: https://chromium-review.googlesource.com/1157876
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579990}
This CL is extracted from https://crrev.com/c/1137851 because that CL
may uncover brokenness and get reverted. Landing these changes
separately minimizes the impact of a potential revert.
Bug: 863724
Change-Id: I9b8888faf7a736f086ca5e496f85a78e0754a0c5
Reviewed-on: https://chromium-review.googlesource.com/1145837
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577289}
The V2 schema was introduced in WebKit on 2011-07-15, in
https://bugs.webkit.org/show_bug.cgi?id=58762. Removing support for the
V1 schema allows us to remove the plumbing for a SQLite feature that no
other part of the codebase uses.
This CL does mean that a hypothetical user who hasn't upgraded Chrome
for 7 years and suddenly upgrades will have their DOM storage reset. We
generally support database schemas up to 2 years old, so this
hypothetical user will most likely start with a clean slate anyways.
Change-Id: I8e21c525a114ed1da856af5c393d62a0ad8cfb1c
Reviewed-on: https://chromium-review.googlesource.com/1146280
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577266}
This CL adds DCHECKs covering a few of the preconditions in
sql::Connection's API and removes sql::TimeSource, which duplicates
base::TickClock.
Bug: none
Change-Id: Ia62874ccf339cf2338e388a45a41d789e3ef89e3
Reviewed-on: https://chromium-review.googlesource.com/1143132
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576589}
0-based ref-counting is deprecated. 1-based counting is described in
//base/memory/ref_counted.h. In summary, instaces must be created by
base::MakeRefCounted<T> or (less ideal) base::AdoptRef(new T).
This CL switches Connection::StatementRef to 1-based ref-counting, and
std::move()'s scoped_refptr<StatementRef> in a few places where that
avoids refcount churn.
Bug: none
Change-Id: I14755dc2482f18c8a9582066104f346c2873941d
Reviewed-on: https://chromium-review.googlesource.com/1137848
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575876}
This is necessary to make these DLOG statments non-fatal in
Albatross builds.
Bug: 596231
Change-Id: I7634aad4eb74f2a0736f9e5775c6198368151c55
Reviewed-on: https://chromium-review.googlesource.com/679254
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504164}
This CL fixes a couple of style nits pointed out by lint in login_database.*, and also changes
sql::Statement::ColumnBlobAsString to const.
BUG=413020
Review-Url: https://codereview.chromium.org/2133173003
Cr-Commit-Position: refs/heads/master@{#405089}
Remove StringToLowerASCII. This removes the template and makes a consistent call that can take a string piece.
http_response_headers.cc in HttpResponseHeaders::RemoveHeaderLine there seemed to be a bug where it did
std::string old_header_name_lowercase(name);
where it actually meant
std::string old_header_name_lowercase(old_header_name);
I fixed this.
There were a number of cases where url.host() or url.scheme() was passed to ToLowerASCII. These are already guaranteed lower-case, so I removed the call.
There were a number of cases in net that did return ToLowerASCII().c_str() when the return type is a std::string, which is unnecessary and wasteful. I removed the c_str() call.
NOPRESUBMIT=true
(for touching code with wstrings)
Review URL: https://codereview.chromium.org/1282363003
Cr-Commit-Position: refs/heads/master@{#342869}
Generate stats for how many SQL statements are executed, how many
results they return, and how many changes they make. Generate timing
values for how long is spent doing all queries, doing updating
operations, doing autocommit updates, and commiting transactions.
The goal of these metrics is to quantify results of decisions like
enabling write-ahead log or memory-mapped I/O.
BUG=489788,489444
Review URL: https://codereview.chromium.org/1145833002
Cr-Commit-Position: refs/heads/master@{#332503}
* Use the standard integer types from stdint.h instead.
* Use macros.h for the DISALLOW_COPY_AND_ASSIGN macro.
BUG=138542
TEST=sql_unittests
R=shess@chromium.org
Review URL: https://codereview.chromium.org/1133053004
Cr-Commit-Position: refs/heads/master@{#329256}
DCHECK if any Bind*() calls occur after Step() has been called. The
bind cannot be implemented at that point, so most likely the calling
code has an error, such as a forgotted Reset().
Also DCHECK if Run() is called after Run() or Step() without an
intervening Reset(). Such a calling sequence is unlikely to be
intentional, because the results of calling Run() twice aren't
defined.
BUG=309759
Review URL: https://codereview.chromium.org/40733003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@231098 0039d316-1c4b-4281-b951-d872f2087c98
Log lines like:
ERROR:connection.cc(1007)] sqlite error 266, errno 5: disk I/O error
would be ever so much more useful if they indicated which database
they were associated with. This logs the histogram tag, which indicates
the logical database (the code which owns this connection). [As
opposed to the filesystem name.]
BUG=none
Review URL: https://codereview.chromium.org/24638002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@225518 0039d316-1c4b-4281-b951-d872f2087c98
This patch was generated by running the empty_string clang tool
across the Chromium Linux compilation database. Implicitly or
explicitly constructing std::string() with a "" argument is
inefficient as the caller needs to emit extra instructions to
pass an argument, and the constructor needlessly copies a byte
into internal storage. Rewriting these instances to simply call
the default constructor appears to save ~14-18 kilobytes on an
optimized release build.
BUG=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193020
Review URL: https://codereview.chromium.org/13145003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193040 0039d316-1c4b-4281-b951-d872f2087c98
This patch was generated by running the empty_string clang tool
across the Chromium Linux compilation database. Implicitly or
explicitly constructing std::string() with a "" argument is
inefficient as the caller needs to emit extra instructions to
pass an argument, and the constructor needlessly copies a byte
into internal storage. Rewriting these instances to simply call
the default constructor appears to save ~14-18 kilobytes on an
optimized release build.
BUG=none
Review URL: https://codereview.chromium.org/13145003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193020 0039d316-1c4b-4281-b951-d872f2087c98
Raze() clears out the database, but cannot be called within a
transaction. Close() can only be called once all statements have
cleared. Error callbacks happen while executing statements (thus
often in a transaction, and at least one statement is outstanding).
RazeAndClose() forcibly breaks outstanding transactions, calls Raze()
to clear the database, then calls Close() to close the handle. All
future operations against the database should fail safely (without
affecting storage and without crashing).
BUG=166419, 136846
Review URL: https://chromiumcodereview.appspot.com/12096073
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181152 0039d316-1c4b-4281-b951-d872f2087c98
Also expanded scope of ScopedAllowIO in
SQLiteServerBoundCertStore::Backend::Load() to cover SQLite functions.
And added ScopedAllowIO to PasswordStoreFactory::BuildServiceInstanceFor() --
it calls LoginDatabase::Init() which should be executed on DB thread.
This is a reland of
https://src.chromium.org/viewvc/chrome?view=rev&revision=147309
which was reverted because of missing ScopedAllowIO in PasswordStoreFactory.
Patch from Pavel Ivanov <paivanof@gmail.com>
BUG=75232, 52909, 137961, 138903
TEST=no test fails with message "Function marked as IO-only was called from a thread that disallows IO!"
Review URL: https://chromiumcodereview.appspot.com/10824008
Patch from Pavel Ivanov <paivanof@gmail.com>.
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148788 0039d316-1c4b-4281-b951-d872f2087c98
Expand scope of ScopedAllowIO in the only place where SQLite functions are used on UI thread.
Patch from Pavel Ivanov <paivanof@gmail.com>
BUG=75232,52909
TEST=no test fails with message "Function marked as IO-only was called from a thread that disallows IO!"
Review URL: https://chromiumcodereview.appspot.com/10540155
Patch from Pavel Ivanov <paivanof@gmail.com>.
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@147309 0039d316-1c4b-4281-b951-d872f2087c98
It is used to support the Andorid' sqlite cursor feature which could move the cursor around the result set.
BUG=
TEST=Added a new test.
TBR=agl,akalin,michaeln
Review URL: http://codereview.chromium.org/10171014
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133985 0039d316-1c4b-4281-b951-d872f2087c98
sql::Statement maintains a weak ref to the associated sql::Connection,
meaning that if the database and statement are destructed in the wrong
order, a use-after-free can result. sql::Statement::Clear() allows
resetting the statement to the default-constructed state.
BUG=111376
TEST=fewer crashes.
Review URL: http://codereview.chromium.org/9418021
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@122430 0039d316-1c4b-4281-b951-d872f2087c98
Android does not have toLower in it's <algorithm>, use
StringToLowerASCII instead.
Review URL: http://codereview.chromium.org/9384004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@121505 0039d316-1c4b-4281-b951-d872f2087c98
Add a class that can read/write a local storage database in the same format as WebCore's StorageArea.
Also add a method to sql::Statement to query the declared type of a column and a method to return a string16 from a Blob.
BUG=106763
Review URL: http://codereview.chromium.org/9159020
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@121442 0039d316-1c4b-4281-b951-d872f2087c98
Pulls out the core of gbillock's http://codereview.chromium.org/8283002/
- Move NOTREACHED and similar checks into the sql:: implementation code.
- Add malformed SQL checks to Connection::Execute.
- Add SQL-checking convenience methods to Connection.
The general idea is that the sql:: framework assumes valid statements,
rather than having client code contain scattered ad-hoc (and thus
inconsistent) checks.
This version puts back Statement operator overloading and loosy-goosy
Execute() calls to allow other code to be updated in small batches.
R=gbillock@chromium.org,jhawkins@chromium.org,dhollowa@chromium.org
BUG=none
TEST=sql_unittests,unit_tests:*Table*.*
Review URL: http://codereview.chromium.org/8899012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114118 0039d316-1c4b-4281-b951-d872f2087c98
I can't remove app/app.gyp and app/app_base.gypi yet because they are referenced
by third_party gyp files :(
BUG=72317
TEST=None
R=rsesek@chromium.org
move app/sql to sql
Review URL: http://codereview.chromium.org/7353026
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@93069 0039d316-1c4b-4281-b951-d872f2087c98
