Switch SQLite to RAM-only temporary storage |
|||||
Issue descriptionThis bug tracks switching the SQLITE_TEMP_STORE compile-time option to 3 (memory-only) on all Chrome platforms. The change will be introduced by a Finch experiment, for insurance against unexpected memory usage increases. From https://www.sqlite.org/tempfiles.html#tempstore -- "The temporary files associated with transaction control, namely the rollback journal, master journal, write-ahead log (WAL) files, and shared-memory files, are always written to disk. But the other kinds of temporary files might be stored in memory only and never written to disk." https://www.sqlite.org/tempfiles.html#temporary_file_storage_locations describes the possible locations. The most likely locations are TMPDIR, /var/tmp and /usr/tmp. This causes two problems: 1) The filesystem hosting the temporary directory might be too small -- see https://crbug.com/43939 2) The temporary directory is outside the user's home directory. SQLite temporary data might contain privacy-sensitive information, which is leaked to other users of the same machine. While this doesn't fall under Chrome's threat model, it's still unfortunate. Asides from the issues above, switching to SQLITE_TEMP_STORE=3 will reduce the inconsistencies between desktop and Android. The latter already uses this option.
,
Aug 18
,
Aug 20
>>> Asides from the issues above, switching to SQLITE_TEMP_STORE=3 will reduce the inconsistencies between desktop and Android. The latter already uses this option. >>> If that's true, then why is this bug/experiment targeting Android? Shouldn't this be a no-op for Android. And what about Fuchsia?
,
Aug 20
I misclicked and got Android instead of Linux. Thanks for catching!
,
Aug 20
I think Fuchsia is covered by this too. We're shipping our own SQLite everywhere, and only set SQLITE_TEMP_STORAGE to 3 on Android: https://cs.chromium.org/chromium/src/third_party/sqlite/BUILD.gn?q=SQLITE_TEMP_STORE
,
Aug 20
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4c2f3e9227e104b2b81056869aabb219fafbc490 commit 4c2f3e9227e104b2b81056869aabb219fafbc490 Author: Victor Costan <pwnall@chromium.org> Date: Tue Aug 21 04:47:59 2018 sql: Feature flag for in-memory temporary storage. This CL introduces SqlTempStoreMemory flag, which is intended to be managed via Finch. When set, the flag causes sql::Database to run a PRAGMA temp_store=MEMORY [1] query on every opened database. This approach is intended to approximate the effect of building SQLite with the SQLITE_TEMP_STORE=3 [2] macro, to measure the memory consumption impact. Ideally, we'd test the macro directly, but //sql is a core component of Chrome, so we can't load different versions of it based on Finch. [1] https://www.sqlite.org/pragma.html#pragma_temp_store [2] https://www.sqlite.org/compile.html#temp_store Bug: 875538 Change-Id: I537d90d763be1100503ed4bd2ada2ee19eb090bb Reviewed-on: https://chromium-review.googlesource.com/1180530 Reviewed-by: Mark Pearson <mpearson@chromium.org> Reviewed-by: Chris Mumford <cmumford@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#584652} [modify] https://crrev.com/4c2f3e9227e104b2b81056869aabb219fafbc490/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc [modify] https://crrev.com/4c2f3e9227e104b2b81056869aabb219fafbc490/sql/BUILD.gn [modify] https://crrev.com/4c2f3e9227e104b2b81056869aabb219fafbc490/sql/database.cc [modify] https://crrev.com/4c2f3e9227e104b2b81056869aabb219fafbc490/sql/database_unittest.cc [add] https://crrev.com/4c2f3e9227e104b2b81056869aabb219fafbc490/sql/sql_features.cc [add] https://crrev.com/4c2f3e9227e104b2b81056869aabb219fafbc490/sql/sql_features.h
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/346f87188d13f1166ab81786e70f7ee8b7ea437f commit 346f87188d13f1166ab81786e70f7ee8b7ea437f Author: Victor Costan <pwnall@chromium.org> Date: Tue Aug 21 18:57:23 2018 Finch: Add config for SqlTempStoreMemory (SQL temporary storage config). Bug: 875538 Change-Id: Ic0e1f11eb0d0dfcadd6e75ebfad5ffa60d778e2c Reviewed-on: https://chromium-review.googlesource.com/1183786 Reviewed-by: Mark Pearson <mpearson@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#584868} [modify] https://crrev.com/346f87188d13f1166ab81786e70f7ee8b7ea437f/testing/variations/fieldtrial_testing_config.json |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by pwnall@chromium.org
, Aug 18