From eb33623749e694205f87baa6f1f6a36fbf3e8e46 Mon Sep 17 00:00:00 2001
From: Christian Speckner <christian.speckner@mayflower.de>
Date: Sat, 2 Jan 2021 14:17:04 +0100
Subject: [PATCH] Move error handling to SqliteDatabase, performance tuning.

---
 src/common/repository/sqlite/SettingsDb.cxx   | 24 +++++++++++--------
 src/common/repository/sqlite/SettingsDb.hxx   | 14 +++++------
 .../repository/sqlite/SqliteDatabase.cxx      |  2 ++
 src/emucore/OSystem.cxx                       | 19 ++++++---------
 src/emucore/OSystem.hxx                       |  8 +++----
 5 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/src/common/repository/sqlite/SettingsDb.cxx b/src/common/repository/sqlite/SettingsDb.cxx
index d32da3e71..1033da893 100644
--- a/src/common/repository/sqlite/SettingsDb.cxx
+++ b/src/common/repository/sqlite/SettingsDb.cxx
@@ -18,6 +18,10 @@
 #include "SettingsDb.hxx"
 #include "Logger.hxx"
 #include "SqliteError.hxx"
+#include "repository/KeyValueRepositoryNoop.hxx"
+#include "repository/CompositeKeyValueRepositoryNoop.hxx"
+#include "repository/CompositeKVRJsonAdapter.hxx"
+#include "KeyValueRepositorySqlite.hxx"
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 SettingsDb::SettingsDb(const string& databaseDirectory, const string& databaseName)
@@ -27,17 +31,19 @@ SettingsDb::SettingsDb(const string& databaseDirectory, const string& databaseNa
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool SettingsDb::initialize()
+void SettingsDb::initialize()
 {
   try {
     myDb = make_unique<SqliteDatabase>(myDatabaseDirectory, myDatabaseName);
     myDb->initialize();
 
-    mySettingsRepository = make_unique<KeyValueRepositorySqlite>(*myDb, "settings", "setting", "value");
-    mySettingsRepository->initialize();
+    auto settingsRepository = make_unique<KeyValueRepositorySqlite>(*myDb, "settings", "setting", "value");
+    settingsRepository->initialize();
+    mySettingsRepository = std::move(settingsRepository);
 
-    myPropertyRepositoryHost = make_unique<KeyValueRepositorySqlite>(*myDb, "properties", "md5", "properties");
-    myPropertyRepositoryHost->initialize();
+    auto propertyRepositoryHost = make_unique<KeyValueRepositorySqlite>(*myDb, "properties", "md5", "properties");
+    propertyRepositoryHost->initialize();
+    myPropertyRepositoryHost = std::move(propertyRepositoryHost);
 
     myPropertyRepository = make_unique<CompositeKVRJsonAdapter>(*myPropertyRepositoryHost);
   }
@@ -45,11 +51,9 @@ bool SettingsDb::initialize()
     Logger::error("sqlite DB " + databaseFileName() + " failed to initialize: " + err.what());
 
     myDb.reset();
-    mySettingsRepository.reset();
-    myPropertyRepository.reset();
+    myPropertyRepositoryHost.reset();
 
-    return false;
+    mySettingsRepository = make_unique<KeyValueRepositoryNoop>();
+    myPropertyRepository = make_unique<CompositeKeyValueRepositoryNoop>();
   }
-
-  return true;
 }
diff --git a/src/common/repository/sqlite/SettingsDb.hxx b/src/common/repository/sqlite/SettingsDb.hxx
index 317c5a63b..ea97cbd7d 100644
--- a/src/common/repository/sqlite/SettingsDb.hxx
+++ b/src/common/repository/sqlite/SettingsDb.hxx
@@ -20,8 +20,8 @@
 
 #include "bspf.hxx"
 #include "SqliteDatabase.hxx"
-#include "KeyValueRepositorySqlite.hxx"
-#include "repository/CompositeKVRJsonAdapter.hxx"
+#include "repository/KeyValueRepository.hxx"
+#include "repository/CompositeKeyValueRepository.hxx"
 
 class SettingsDb
 {
@@ -29,13 +29,13 @@ class SettingsDb
 
     SettingsDb(const string& databaseDirectory, const string& databaseName);
 
-    bool initialize();
+    void initialize();
 
     KeyValueRepository& settingsRepository() const { return *mySettingsRepository; }
 
     CompositeKeyValueRepository& propertyRepository() const { return *myPropertyRepository; }
 
-    const string& databaseFileName() const { return myDb->fileName(); }
+    const string& databaseFileName() const { return myDatabaseName; }
 
   private:
 
@@ -43,9 +43,9 @@ class SettingsDb
     string myDatabaseName;
 
     shared_ptr<SqliteDatabase> myDb;
-    unique_ptr<KeyValueRepositorySqlite> mySettingsRepository;
-    unique_ptr<KeyValueRepositorySqlite> myPropertyRepositoryHost;
-    unique_ptr<CompositeKVRJsonAdapter> myPropertyRepository;
+    unique_ptr<KeyValueRepositoryAtomic> mySettingsRepository;
+    unique_ptr<KeyValueRepositoryAtomic> myPropertyRepositoryHost;
+    unique_ptr<CompositeKeyValueRepository> myPropertyRepository;
 };
 
 #endif // SETTINGS_DB_HXX
diff --git a/src/common/repository/sqlite/SqliteDatabase.cxx b/src/common/repository/sqlite/SqliteDatabase.cxx
index 4b9912a5a..8c96b70d7 100644
--- a/src/common/repository/sqlite/SqliteDatabase.cxx
+++ b/src/common/repository/sqlite/SqliteDatabase.cxx
@@ -69,9 +69,11 @@ void SqliteDatabase::initialize()
   }
 
   exec("PRAGMA journal_mode=WAL");
+  exec("PRAGMA synchronous=1");
 
   switch (sqlite3_wal_checkpoint_v2(myHandle, nullptr, SQLITE_CHECKPOINT_TRUNCATE, nullptr, nullptr)) {
     case SQLITE_OK:
+    case SQLITE_BUSY:
       break;
 
     case SQLITE_MISUSE:
diff --git a/src/emucore/OSystem.cxx b/src/emucore/OSystem.cxx
index c22be26a2..9608fd5ed 100644
--- a/src/emucore/OSystem.cxx
+++ b/src/emucore/OSystem.cxx
@@ -224,12 +224,11 @@ void OSystem::loadConfig(const Settings::Options& options)
     myHomeDir.makeDir();
 
   mySettingsDb = make_shared<SettingsDb>(myBaseDir.getPath(), "settings");
-  if(!mySettingsDb->initialize())
-    mySettingsDb.reset();
+  mySettingsDb->initialize();
   myConfigFile = FilesystemNode(mySettingsDb->databaseFileName());
 
-  mySettings->setRepository(createSettingsRepository());
-  myPropSet->setRepository(createPropertyRepository());
+  mySettings->setRepository(getSettingsRepository());
+  myPropSet->setRepository(getPropertyRepository());
   mySettings->load(options);
 
   // userDir is NOT affected by '-baseDir'and '-basedirinapp' params
@@ -880,18 +879,14 @@ void OSystem::mainLoop()
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-shared_ptr<KeyValueRepository> OSystem::createSettingsRepository()
+shared_ptr<KeyValueRepository> OSystem::getSettingsRepository()
 {
-    return mySettingsDb
-      ? shared_ptr<KeyValueRepository>(mySettingsDb, &mySettingsDb->settingsRepository())
-      : make_shared<KeyValueRepositoryNoop>();
+    return shared_ptr<KeyValueRepository>(mySettingsDb, &mySettingsDb->settingsRepository());
 }
 
-shared_ptr<CompositeKeyValueRepository> OSystem::createPropertyRepository()
+shared_ptr<CompositeKeyValueRepository> OSystem::getPropertyRepository()
 {
-  return mySettingsDb
-    ? shared_ptr<CompositeKeyValueRepository>(mySettingsDb, &mySettingsDb->propertyRepository())
-    : make_shared<CompositeKeyValueRepositoryNoop>();
+  return shared_ptr<CompositeKeyValueRepository>(mySettingsDb, &mySettingsDb->propertyRepository());
 }
 
 
diff --git a/src/emucore/OSystem.hxx b/src/emucore/OSystem.hxx
index a95d5df24..a0c845eed 100644
--- a/src/emucore/OSystem.hxx
+++ b/src/emucore/OSystem.hxx
@@ -447,12 +447,12 @@ class OSystem
     */
     virtual void stateChanged(EventHandlerState state) { }
 
+    virtual shared_ptr<KeyValueRepository> getSettingsRepository();
+
+    virtual shared_ptr<CompositeKeyValueRepository> getPropertyRepository();
+
   protected:
 
-    virtual shared_ptr<KeyValueRepository> createSettingsRepository();
-
-    virtual shared_ptr<CompositeKeyValueRepository> createPropertyRepository();
-
     //////////////////////////////////////////////////////////////////////
     // The following methods are system-specific and *must* be
     // implemented in derived classes.