sql: Improve documentation for Database::Preload().

base::PreReadFile() was implemented on macOS and iOS. Windows 7
currently has limited enterprise-only support.

Change-Id: Ia53bcf45dc79aa4d92067c6ea4ace234a0bcba70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3519659
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#980294}

sql/database.cc

@@ -366,16 +366,14 @@ void Database::Preload() {
 
   // Maximum number of bytes that will be prefetched from the database.
   //
-  // This limit is very aggressive. Here are the trade-offs involved.
-  // 1) Accessing bytes that weren't preread is very expensive on
-  //    performance-critical databases, so the limit must exceed the expected
-  //    sizes of feature databases.
-  // 2) On some platforms (Windows 7 and, currently, macOS), base::PreReadFile()
-  //    falls back to a synchronous read, and blocks until the entire file is
-  //    read into memory. So, there's a tangible cost to reading data that would
-  //    get evicted before base::PreReadFile() completes. This cost needs to be
-  //    balanced with the benefit reading the entire database at once, and
-  //    avoiding seeks on spinning disks.
+  // This limit is very aggressive. The main trade-off involved is that having
+  // SQLite block on reading from disk has a high impact on Chrome startup cost
+  // for the databases that are on the critical path to startup. So, the limit
+  // must exceed the expected sizes of databases on the critical path.
+  //
+  // On Windows 7, base::PreReadFile() falls back to a synchronous read, and
+  // blocks until the entire file is read into memory. This is a minor factor at
+  // this point, because Chrome has very limited support for Windows 7.
   constexpr int kPreReadSize = 128 * 1024 * 1024;  // 128 MB
   base::PreReadFile(DbPath(), /*is_executable=*/false, kPreReadSize);
 }

sql/database.h

@@ -293,17 +293,17 @@ class COMPONENT_EXPORT(SQL) Database {
   // an uninitialized or already-closed database.
   void Close();
 
-  // Reads the first <cache-size>*<page-size> bytes of the file to prime the
-  // filesystem cache.  This can be more efficient than faulting pages
-  // individually.  Since this involves blocking I/O, it should only be used if
-  // the caller will immediately read a substantial amount of data from the
-  // database.
-  //
-  // TODO(shess): Design a set of histograms or an experiment to inform this
-  // decision.  Preloading should almost always improve later performance
-  // numbers for this database simply because it pulls operations forward, but
-  // if the data isn't actually used soon then preloading just slows down
-  // everything else.
+  // Hints the file system that the database will be accessed soon.
+  //
+  // This method should be called on databases that are on the critical path to
+  // Chrome startup. Informing the filesystem about our expected access pattern
+  // early on reduces the likelihood that we'll be blocked on disk I/O. This has
+  // a high impact on startup time.
+  //
+  // This method should not be used for non-critical databases. While using it
+  // will likely improve micro-benchmarks involving one specific database,
+  // overuse risks randomizing the disk I/O scheduler, slowing down Chrome
+  // startup.
   void Preload();
 
   // Release all non-essential memory associated with this database connection.