From 6747365baa8596632b4a92bf6f9290835cd6596e Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 1 Nov 2021 22:55:47 +0400 Subject: [PATCH 1/3] Enter KiwixApp::NameMapperProxy The new helper class `KiwixApp::NameMapperProxy` serves as a level of indirection for `kiwix::HumanReadableNameMapper`. It will help to solve the problem of updating the mapping when the library changes. --- src/kiwixapp.cpp | 24 +++++++++++++++++++++++- src/kiwixapp.h | 14 +++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/kiwixapp.cpp b/src/kiwixapp.cpp index 59df14d..4b9855c 100644 --- a/src/kiwixapp.cpp +++ b/src/kiwixapp.cpp @@ -15,6 +15,28 @@ #include #include +//////////////////////////////////////////////////////////////////////////////// +// KiwixApp::NameMapperProxy +//////////////////////////////////////////////////////////////////////////////// + +KiwixApp::NameMapperProxy::NameMapperProxy(kiwix::Library& library) + : impl(new kiwix::HumanReadableNameMapper(library, false)) +{} + +std::string KiwixApp::NameMapperProxy::getNameForId(const std::string& id) +{ + return impl->getNameForId(id); +} + +std::string KiwixApp::NameMapperProxy::getIdForName(const std::string& name) +{ + return impl->getIdForName(name); +} + +//////////////////////////////////////////////////////////////////////////////// +// KiwixApp +//////////////////////////////////////////////////////////////////////////////// + KiwixApp::KiwixApp(int& argc, char *argv[]) : QtSingleApplication("kiwix-desktop", argc, argv), m_profile(), @@ -23,7 +45,7 @@ KiwixApp::KiwixApp(int& argc, char *argv[]) mp_downloader(nullptr), mp_manager(nullptr), mp_mainWindow(nullptr), - m_nameMapper(m_library.getKiwixLibrary(), false), + m_nameMapper(m_library.getKiwixLibrary()), m_server(&m_library.getKiwixLibrary(), &m_nameMapper) { try { diff --git a/src/kiwixapp.h b/src/kiwixapp.h index c5a6de3..f200517 100644 --- a/src/kiwixapp.h +++ b/src/kiwixapp.h @@ -104,6 +104,18 @@ protected: void createAction(); void postInit(); +private: // types + class NameMapperProxy : public kiwix::NameMapper { + public: + explicit NameMapperProxy(kiwix::Library& library); + + virtual std::string getNameForId(const std::string& id); + virtual std::string getIdForName(const std::string& name); + + std::shared_ptr impl; + }; + + private: QTranslator m_qtTranslator, m_appTranslator; SettingsManager m_settingsManager; @@ -116,7 +128,7 @@ private: TabBar* mp_tabWidget; SideBarType m_currentSideType; QErrorMessage* mp_errorDialog; - kiwix::HumanReadableNameMapper m_nameMapper; + NameMapperProxy m_nameMapper; kiwix::Server m_server; Translation m_translation; From df6644a7ffaa4cc274861fc4fe6f130d70d18d84 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 1 Nov 2021 23:14:34 +0400 Subject: [PATCH 2/3] Library updates are propagated to the NameMapper --- src/kiwixapp.cpp | 23 ++++++++++++++++++----- src/kiwixapp.h | 7 ++++++- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/kiwixapp.cpp b/src/kiwixapp.cpp index 4b9855c..839fed1 100644 --- a/src/kiwixapp.cpp +++ b/src/kiwixapp.cpp @@ -19,18 +19,25 @@ // KiwixApp::NameMapperProxy //////////////////////////////////////////////////////////////////////////////// -KiwixApp::NameMapperProxy::NameMapperProxy(kiwix::Library& library) - : impl(new kiwix::HumanReadableNameMapper(library, false)) -{} +KiwixApp::NameMapperProxy::NameMapperProxy(kiwix::Library& lib) + : library(lib) +{ + update(); +} + +void KiwixApp::NameMapperProxy::update() +{ + nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); +} std::string KiwixApp::NameMapperProxy::getNameForId(const std::string& id) { - return impl->getNameForId(id); + return nameMapper->getNameForId(id); } std::string KiwixApp::NameMapperProxy::getIdForName(const std::string& name) { - return impl->getIdForName(name); + return nameMapper->getIdForName(name); } //////////////////////////////////////////////////////////////////////////////// @@ -433,6 +440,7 @@ void KiwixApp::postInit() { [=](const QString& title) { emit currentTitleChanged(title); }); connect(mp_tabWidget, &TabBar::libraryPageDisplayed, this, &KiwixApp::disableItemsOnLibraryPage); emit(m_library.booksChanged()); + connect(&m_library, &Library::booksChanged, this, &KiwixApp::updateNameMapper); disableItemsOnLibraryPage(true); } @@ -444,3 +452,8 @@ void KiwixApp::disableItemsOnLibraryPage(bool libraryDisplayed) KiwixApp::instance()->getAction(KiwixApp::ZoomOutAction)->setDisabled(libraryDisplayed); KiwixApp::instance()->getAction(KiwixApp::ZoomResetAction)->setDisabled(libraryDisplayed); } + +void KiwixApp::updateNameMapper() +{ + m_nameMapper.update(); +} diff --git a/src/kiwixapp.h b/src/kiwixapp.h index f200517..b324de1 100644 --- a/src/kiwixapp.h +++ b/src/kiwixapp.h @@ -99,6 +99,7 @@ public slots: void toggleSideBar(KiwixApp::SideBarType type); void printPage(); void disableItemsOnLibraryPage(bool displayed); + void updateNameMapper(); protected: void createAction(); @@ -112,7 +113,11 @@ private: // types virtual std::string getNameForId(const std::string& id); virtual std::string getIdForName(const std::string& name); - std::shared_ptr impl; + void update(); + + private: + kiwix::Library& library; + std::shared_ptr nameMapper; }; From 44ddebf90cd1a19b371c4fbad7c87a736b24347a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 1 Nov 2021 23:44:24 +0400 Subject: [PATCH 3/3] Thread-safe KiwixApp::NameMapperProxy The chosen synchronization scheme between update() and getNameForId()/getIdForName() optimizes the duration of the critical sections - the getNameForId()/getIdForName() operations are not locked for their full duration of execution. --- src/kiwixapp.cpp | 22 +++++++++++++++++++--- src/kiwixapp.h | 9 ++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/kiwixapp.cpp b/src/kiwixapp.cpp index 839fed1..83318e5 100644 --- a/src/kiwixapp.cpp +++ b/src/kiwixapp.cpp @@ -27,17 +27,33 @@ KiwixApp::NameMapperProxy::NameMapperProxy(kiwix::Library& lib) void KiwixApp::NameMapperProxy::update() { - nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); + const auto newNameMapper = new kiwix::HumanReadableNameMapper(library, false); + std::lock_guard lock(mutex); + nameMapper.reset(newNameMapper); +} + +KiwixApp::NameMapperProxy::NameMapperHandle +KiwixApp::NameMapperProxy::currentNameMapper() const +{ + // Return a copy of the handle to the current NameMapper object. It will + // ensure that the object survives any call to NameMapperProxy::update() + // made before the completion of any pending operation on that object. + std::lock_guard lock(mutex); + return nameMapper; } std::string KiwixApp::NameMapperProxy::getNameForId(const std::string& id) { - return nameMapper->getNameForId(id); + // Ensure that the current nameMapper object survives a concurrent call + // to NameMapperProxy::update() + return currentNameMapper()->getNameForId(id); } std::string KiwixApp::NameMapperProxy::getIdForName(const std::string& name) { - return nameMapper->getIdForName(name); + // Ensure that the current nameMapper object survives a concurrent call + // to NameMapperProxy::update() + return currentNameMapper()->getIdForName(name); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/kiwixapp.h b/src/kiwixapp.h index b324de1..098e79a 100644 --- a/src/kiwixapp.h +++ b/src/kiwixapp.h @@ -18,6 +18,8 @@ #include #include +#include + class KiwixApp : public QtSingleApplication { @@ -107,6 +109,7 @@ protected: private: // types class NameMapperProxy : public kiwix::NameMapper { + typedef std::shared_ptr NameMapperHandle; public: explicit NameMapperProxy(kiwix::Library& library); @@ -116,8 +119,12 @@ private: // types void update(); private: + NameMapperHandle currentNameMapper() const; + + private: + mutable std::mutex mutex; kiwix::Library& library; - std::shared_ptr nameMapper; + NameMapperHandle nameMapper; };