Bug 1654192 - Part 2: Update xulstore to use RKV in safe mode, r=nanj,perftest-reviewers,AlexandruIonescu

Differential Revision: https://phabricator.services.mozilla.com/D85315

browser/base/content/test/performance/browser_startup_mainthreadio.js

@@ -219,8 +219,7 @@ const startupPhases = {
       path: "ProfD:xulstore/data.mdb",
       condition: WIN,
       read: 1,
-      write: 3,
-      fsync: 1,
+      write: 1,
     },
   ],
 
@@ -281,12 +280,6 @@ const startupPhases = {
       condition: WIN,
       stat: 1,
     },
-    {
-      // bug 1546838
-      path: "ProfD:xulstore/data.mdb",
-      condition: WIN,
-      read: 2,
-    },
   ],
 
   // We reach this phase right after showing the first browser window.
@@ -360,7 +353,7 @@ const startupPhases = {
       // bug 1546838
       path: "ProfD:xulstore/data.mdb",
       condition: MAC,
-      write: 3,
+      write: 1,
     },
   ],
 

testing/talos/talos/xtalos/xperf_whitelist.json

@@ -608,6 +608,12 @@
     "minbytes": 0,
     "maxbytes": 608
   },
+  "{profile}\\xulstore\\data.safe.bin": {
+    "mincount": 0,
+    "maxcount": 4,
+    "minbytes": 0,
+    "maxbytes": 574
+  },
   "{talos}\\talos\\tests\\{tp5n_files}": {
     "mincount": 0,
     "maxcount": 2,

toolkit/components/xulstore/src/error.rs

@@ -5,7 +5,7 @@
 use nserror::{
     nsresult, NS_ERROR_FAILURE, NS_ERROR_ILLEGAL_VALUE, NS_ERROR_NOT_AVAILABLE, NS_ERROR_UNEXPECTED,
 };
-use rkv::{migrate::MigrateError as RkvMigrateError, StoreError as RkvStoreError};
+use rkv::{MigrateError as RkvMigrateError, StoreError as RkvStoreError};
 use serde_json::Error as SerdeJsonError;
 use std::{io::Error as IoError, str::Utf8Error, string::FromUtf16Error, sync::PoisonError};
 

toolkit/components/xulstore/src/lib.rs

@@ -8,7 +8,6 @@ extern crate cstr;
 #[macro_use]
 extern crate failure;
 extern crate libc;
-extern crate lmdb;
 #[macro_use]
 extern crate log;
 extern crate moz_task;

toolkit/components/xulstore/src/persist.rs

@@ -19,7 +19,6 @@ use crate::{
     statics::get_database,
 };
 use crossbeam_utils::atomic::AtomicCell;
-use lmdb::Error as LmdbError;
 use moz_task::{dispatch_background_task_with_options, DispatchOptions, Task, TaskRunnable};
 use nserror::nsresult;
 use once_cell::sync::Lazy;
@@ -73,7 +72,7 @@ fn sync_persist() -> XULStoreResult<()> {
                     // to remove a value that doesn't exist in the store,
                     // so we ignore the error (although in this case the key
                     // should exist, since it was in the cache!).
-                    Err(RkvStoreError::LmdbError(LmdbError::NotFound)) => {
+                    Err(RkvStoreError::KeyValuePairNotFound) => {
                         warn!("tried to remove key that isn't in the store");
                     }
 

toolkit/components/xulstore/src/statics.rs

@@ -7,21 +7,22 @@ use crate::{
     ffi::ProfileChangeObserver,
     make_key, SEPARATOR,
 };
-use lmdb::Error as LmdbError;
 use moz_task::is_main_thread;
 use nsstring::nsString;
 use once_cell::sync::Lazy;
-use rkv::{migrate::Migrator, Rkv, SingleStore, StoreError, StoreOptions, Value};
+use rkv::backend::{SafeMode, SafeModeDatabase, SafeModeEnvironment};
+use rkv::{Migrator, StoreOptions, Value};
 use std::{
     collections::BTreeMap,
-    fs::{copy, create_dir_all, remove_file, File},
+    fs::{create_dir_all, remove_file, File},
     path::PathBuf,
     str,
     sync::Mutex,
 };
-use tempfile::tempdir;
 use xpcom::{interfaces::nsIFile, XpCom};
 
+type Rkv = rkv::Rkv<SafeModeEnvironment>;
+type SingleStore = rkv::SingleStore<SafeModeDatabase>;
 type XULStoreCache = BTreeMap<String, BTreeMap<String, BTreeMap<String, String>>>;
 
 pub struct Database {
@@ -47,27 +48,10 @@ pub(crate) fn get_database() -> XULStoreResult<Database> {
     let xulstore_dir = get_xulstore_dir()?;
     let xulstore_path = xulstore_dir.as_path();
 
-    let env = match Rkv::new(xulstore_path) {
-        Ok(env) => Ok(env),
-        Err(StoreError::LmdbError(LmdbError::Invalid)) => {
-            let temp_env = tempdir()?;
-            let mut migrator = Migrator::new(&xulstore_path)?;
-            migrator.migrate(temp_env.path())?;
-            copy(
-                temp_env.path().join("data.mdb"),
-                xulstore_path.join("data.mdb"),
-            )?;
-            copy(
-                temp_env.path().join("lock.mdb"),
-                xulstore_path.join("lock.mdb"),
-            )?;
-            Rkv::new(xulstore_path)
-        }
-        Err(err) => Err(err),
-    }?;
+    let env = Rkv::new::<SafeMode>(xulstore_path)?;
+    Migrator::easy_migrate_lmdb_to_safe_mode(xulstore_path, &env)?;
 
     let store = env.open_single("db", StoreOptions::create())?;
-
     Ok(Database::new(env, store))
 }
 
@@ -186,14 +170,11 @@ fn cache_data() -> XULStoreResult<XULStoreCache> {
 
     for result in iterator {
         let (key, value): (&str, String) = match result {
-            Ok((key, value)) => {
-                assert!(value.is_some(), "iterated key has value");
-                match (str::from_utf8(&key), unwrap_value(&value)) {
-                    (Ok(key), Ok(value)) => (key, value),
-                    (Err(err), _) => return Err(err.into()),
-                    (_, Err(err)) => return Err(err),
-                }
-            }
+            Ok((key, value)) => match (str::from_utf8(&key), unwrap_value(&value)) {
+                (Ok(key), Ok(value)) => (key, value),
+                (Err(err), _) => return Err(err.into()),
+                (_, Err(err)) => return Err(err),
+            },
             Err(err) => return Err(err.into()),
         };
 
@@ -255,17 +236,13 @@ fn maybe_migrate_data(env: &Rkv, store: SingleStore) {
     .unwrap_or_else(|err| error!("error migrating data: {}", err));
 }
 
-fn unwrap_value(value: &Option<Value>) -> XULStoreResult<String> {
+fn unwrap_value(value: &Value) -> XULStoreResult<String> {
     match value {
-        Some(Value::Str(val)) => Ok(val.to_string()),
-
-        // Per the XULStore API, return an empty string if the value
-        // isn't found.
-        None => Ok(String::new()),
+        Value::Str(val) => Ok(val.to_string()),
 
         // This should never happen, but it could happen in theory
         // if someone writes a different kind of value into the store
         // using a more general API (kvstore, rkv, LMDB).
-        Some(_) => Err(XULStoreError::UnexpectedValue),
+        _ => Err(XULStoreError::UnexpectedValue),
     }
 }