Disable unused SQLite features |
||
Issue descriptionSQLite supports a vast array of compiler flags [1] that disable unused features. The vast majority of these flags start with SQLITE_OMIT_ or SQLITE_DISABLE_. We have already enabled quite a few flags [2] that covered clearly unused features. This bug tracks adding remaining flags, which cover features whose use is difficult to evaluate. Removing unused features reduces binary size, and simplifies debugging bugs and crashers by reducing the amount of active code. SQL features. [ ] SQLITE_OMIT_ANALYZE - bad idea, but may be used by WebSQL code [ ] SQLITE_OMIT_DATETIME_FUNCS - probably used by WebSQL code [x] SQLITE_OMIT_EXPLAIN - not guaranteed to be stable [ ] SQLITE_ENABLE_FTS3 - definitely used by WebSQL [x] SQLITE_OMIT_REINDEX - noop, as we don't define custom collations [ ] SQLITE_OMIT_VIRTUALTABLE - if we can get rid of FTS3 and the recovery module API flags. Can be removed if Chrome still compiles. [ ] SQLITE_OMIT_AUTHORIZATION - after we remove WebSQL [ ] SQLITE_OMIT_UTF16 - after we remove WebSQL Tuning flags. [x] SQLITE_OMIT_LOOKASIDE - we already set SQLITE_DEFAULT_LOOKASIDE=0,0 Pragma flags. WebSQL can't use PRAGMA. Chrome usage can be checked by code-searching against the list in [3]. [ ] SQLITE_OMIT_FLAG_PRAGMAS [ ] SQLITE_OMIT_PAGER_PRAGMAS [ ] SQLITE_OMIT_PRAGMA - would require getting alternative impls for the pragmas we use [ ] SQLITE_OMIT_SCHEMA_PRAGMAS [ ] SQLITE_OMIT_SCHEMA_VERSION_PRAGMAS [1] https://www.sqlite.org/compile.html [2] https://cs.chromium.org/chromium/src/third_party/sqlite/BUILD.gn [3] https://cs.chromium.org/chromium/src/third_party/sqlite/src/src/pragma.h
,
Dec 2
PRAGMA foreign_keys [1] appears to be used in a few places in the source code [2]. This blocks using SQLITE_OMIT_FLAG_PRAGMAS. sqlite3_db_config with SQLITE_DBCONFIG_ENABLE_FKEY [3] can be used instead. [1] https://www.sqlite.org/foreignkeys.html#fk_enable [2] https://cs.chromium.org/search/?q=-file:third_party/sqlite+-file:third_party/perl+-file:third_party/android_ndk+-file:third_party/android_tools+%22PRAGMA+foreign_keys%22&p=7&sq=package:chromium&type=cs [3] https://www.sqlite.org/capi3ref.html#sqlite3_db_config
,
Dec 2
PRAGMA locking_mode [1] is used in the source code [2]. This blocks using SQLITE_OMIT_PAGER_PRAGMAS. We should build with SQLITE_DEFAULT_LOCKING_MODE=1 (exclusive by default) and not use the shared mode. Shared mode should only be needed by WebSQL, which may access the same DB from multiple renderers. [1] https://www.sqlite.org/pragma.html#pragma_locking_mode [2] https://cs.chromium.org/search/?q=%22PRAGMA+locking_mode%22+-file:third_party/sqlite+-file:third_party/perl+-file:third_party/google_toolbox_for_mac+-file:third_party/android_ndk+-file:third_party/android_tools&sq=package:chromium&type=cs [3] https://www.sqlite.org/compile.html#default_locking_mode
,
Dec 2
PRAGMA mmap_size [1] is used in the source code [2] to enable and disable memory-mapped file access. This blocks using SQLITE_OMIT_PAGER_PRAGMAS. Instead, we can use sqlite3_file_control() [3] with SQLITE_FCNTL_MMAP_SIZE. [1] https://www.sqlite.org/pragma.html#pragma_mmap_size [2] https://cs.chromium.org/search/?q=%22PRAGMA+mmap_size%22+-file:third_party/sqlite+-file:third_party/perl+-file:third_party/google_toolbox_for_mac+-file:third_party/android_ndk+-file:third_party/android_tools&sq=package:chromium&type=cs [3] https://www.sqlite.org/capi3ref.html#sqlite3_file_control [4] https://www.sqlite.org/capi3ref.html#sqlitefcntlmmapsize
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dc095f4061ca2d1ea214b4339e08b132d789f2d5 commit dc095f4061ca2d1ea214b4339e08b132d789f2d5 Author: Victor Costan <pwnall@chromium.org> Date: Wed Dec 05 02:39:44 2018 sqlite: Fully disable the lookaside allocator. We already set SQLITE_DEFAULT_LOOKASIDE to "0,0", which effectively disables SQLite's arena memory allocator by giving it no buffers to work with. SQLITE_OMIT_LOOKASIDE removes the logic from the binary altogether. Bug: 910955 Change-Id: Iaef538fe34aec37e4e0668891284ead52df7a2e8 Reviewed-on: https://chromium-review.googlesource.com/c/1358000 Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#613827} [modify] https://crrev.com/dc095f4061ca2d1ea214b4339e08b132d789f2d5/third_party/sqlite/BUILD.gn
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10 commit f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10 Author: Victor Costan <pwnall@chromium.org> Date: Wed Dec 05 04:35:53 2018 sql: Remove redundant PRAGMA secure_delete=ON. Chrome builds SQLite with SQLITE_SECURE_DELETE [1], so the default value for the secure_delete PRAGMA [2] is already true. [1] https://www.sqlite.org/compile.html#secure_delete [2] https://www.sqlite.org/pragma.html#pragma_secure_delete Bug: 910955 Change-Id: I27ef32770a980c8c6030675e79698d7a31e47a9d Reviewed-on: https://chromium-review.googlesource.com/c/1357824 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Cr-Commit-Position: refs/heads/master@{#613866} [modify] https://crrev.com/f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10/sql/database.cc [modify] https://crrev.com/f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10/third_party/blink/web_tests/storage/websql/test-authorizer-expected.txt [modify] https://crrev.com/f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10/third_party/sqlite/BUILD.gn [modify] https://crrev.com/f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10/third_party/sqlite/amalgamation/sqlite3.c [modify] https://crrev.com/f1e9443b7cb1558547ab5fe61a49b5d2f92c4c10/third_party/sqlite/scripts/generate_amalgamation.sh
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/52bef810b53bc1dad2498ee59f4cf540981a7469 commit 52bef810b53bc1dad2498ee59f4cf540981a7469 Author: Victor Costan <pwnall@chromium.org> Date: Wed Dec 05 07:41:49 2018 sql: Use SQLite API instead of PRAGMA on memory pressure. The SQLite API sqlite3_db_release_memory() [1] promises to release all unnecessary memory associated with a DB, not just unused entries in the page cache. PRAGMA cache_size [2] only impacts the page cache. [1] https://www.sqlite.org/c3ref/db_release_memory.html [2] https://www.sqlite.org/pragma.html#pragma_cache_size Bug: 910955 Change-Id: I3f038e5492c6fd4c123520fd428f997d1ae238ab Reviewed-on: https://chromium-review.googlesource.com/c/1357921 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Cr-Commit-Position: refs/heads/master@{#613900} [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/components/history/core/browser/history_backend.cc [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/components/history/core/browser/history_database.cc [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/components/history/core/browser/history_database.h [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/components/history/core/browser/thumbnail_database.cc [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/components/history/core/browser/thumbnail_database.h [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/content/browser/dom_storage/dom_storage_database.cc [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/sql/database.cc [modify] https://crrev.com/52bef810b53bc1dad2498ee59f4cf540981a7469/sql/database.h
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ff47e97f77c94e87fc4f1c1e68e33e3e7dce1b1 commit 1ff47e97f77c94e87fc4f1c1e68e33e3e7dce1b1 Author: Victor Costan <pwnall@chromium.org> Date: Fri Dec 07 11:10:43 2018 sql: Use SQLite API instead of PRAGMA to check for column existence. sql::Database::DoesColumnExist currently compiles and runs a "PRAGMA table_info" [1] SQL query and iterates over the result looking for the desired column name. This is more brittle and expensive than using the the sqlite3_table_column_metadata() [2] API, which pulls schema data directly, bypassing query compilation and execution. This CL introduces a behavior change -- the previous PRAGMA table_info implementation works for views [3, 4], whereas sqlite3_table_column_metadata() only works for tables. The behavior change is not visible, because Chrome currently uses views in a single feature [5], namely extensions [6]. DoesColumnExist should only be used for migrations, which should operate on concrete tables. In steady-state, each feature should have the database schema migrated to the current version, so column existence is known statically. So, removing support for views in DoesColumnExist() is acceptable. [1] https://www.sqlite.org/pragma.html#pragma_table_info [2] https://www.sqlite.org/c3ref/table_column_metadata.html [3] https://www.sqlite.org/matrix/pragma.html [4] https://www.sqlite.org/matrix/matrix_dpragma.html#R-35224-32827-55933-44478-17553-06933-55583-57822 [5] https://cs.chromium.org/search/?q=file:%5C.cc+sql::+VIEW+case:yes [6] https://cs.chromium.org/chromium/src/chrome/browser/extensions/activity_log/counting_policy.cc?q=VIEW+case:yes Bug: 910955 Change-Id: I44474585e6bb2523b5cd91c76c3444058822e601 Reviewed-on: https://chromium-review.googlesource.com/c/1357825 Reviewed-by: Chris Mumford <cmumford@google.com> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#614663} [modify] https://crrev.com/1ff47e97f77c94e87fc4f1c1e68e33e3e7dce1b1/sql/database.cc [modify] https://crrev.com/1ff47e97f77c94e87fc4f1c1e68e33e3e7dce1b1/sql/database.h
,
Dec 7
|
||
►
Sign in to add a comment |
||
Comment 1 by pwnall@chromium.org
, Dec 2