From 55c7c62c7c633982e75eb0e55ca510b0aa99b13b Mon Sep 17 00:00:00 2001 From: Martchus Date: Tue, 31 May 2022 20:54:02 +0200 Subject: [PATCH] Speed up returning build actions table by avoiding deserializing whole obj --- librepomgr/buildactions/buildaction.cpp | 2 +- librepomgr/buildactions/buildaction.h | 93 ++++++++---------------- librepomgr/serversetup.cpp | 8 +- librepomgr/serversetup.h | 7 +- librepomgr/tests/webapi.cpp | 2 +- librepomgr/webapi/routes_buildaction.cpp | 4 +- 6 files changed, 43 insertions(+), 73 deletions(-) diff --git a/librepomgr/buildactions/buildaction.cpp b/librepomgr/buildactions/buildaction.cpp index f1eed50..1613a9f 100644 --- a/librepomgr/buildactions/buildaction.cpp +++ b/librepomgr/buildactions/buildaction.cpp @@ -198,7 +198,7 @@ bool InternalBuildAction::reportAbortedIfAborted() } BuildAction::BuildAction(IdType id, ServiceSetup *setup) noexcept - : id(id) + : BuildActionBase(id) , m_log(this) , m_setup(setup) , m_stopHandler(std::bind(&BuildAction::terminateOngoingBuildProcesses, this)) diff --git a/librepomgr/buildactions/buildaction.h b/librepomgr/buildactions/buildaction.h index 5b5d3f0..44ffb45 100644 --- a/librepomgr/buildactions/buildaction.h +++ b/librepomgr/buildactions/buildaction.h @@ -158,8 +158,35 @@ struct LIBREPOMGR_EXPORT BuildActionBase : public ReflectiveRapidJSON::JsonSeria public ReflectiveRapidJSON::BinarySerializable { using IdType = BuildActionIdType; static constexpr IdType invalidId = std::numeric_limits::max(); + explicit BuildActionBase(IdType id = invalidId); + + bool isScheduled() const; + bool isExecuting() const; + bool isDone() const; + bool hasSucceeded() const; + + IdType id; + BuildActionType type = BuildActionType::Invalid; + std::string taskName; + std::string templateName; + BuildActionStatus status = BuildActionStatus::Created; + BuildActionResult result = BuildActionResult::None; + CppUtilities::DateTime created = CppUtilities::DateTime::gmtNow(); + CppUtilities::DateTime started; + CppUtilities::DateTime finished; + std::vector startAfter; + std::string directory; + std::vector sourceDbs, destinationDbs; + std::vector packageNames; + BuildActionFlagType flags = noBuildActionFlags; + std::unordered_map settings; }; +inline BuildActionBase::BuildActionBase(IdType id) + : id(id) +{ +} + struct LIBREPOMGR_EXPORT BuildAction : public BuildActionBase, public std::enable_shared_from_this, public ReflectiveRapidJSON::JsonSerializable, @@ -178,10 +205,6 @@ public: explicit BuildAction(IdType id = invalidId, ServiceSetup *setup = nullptr) noexcept; BuildAction &operator=(BuildAction &&other); ~BuildAction(); - bool isScheduled() const; - bool isExecuting() const; - bool isDone() const; - bool hasSucceeded() const; static bool haveSucceeded(const std::vector> &buildActions); bool isAborted() const; const std::atomic_bool &aborted() const; @@ -216,22 +239,6 @@ private: LibPkg::StorageID conclude(BuildActionResult result); public: - IdType id; - BuildActionType type = BuildActionType::Invalid; - std::string taskName; - std::string templateName; - BuildActionStatus status = BuildActionStatus::Created; - BuildActionResult result = BuildActionResult::None; - CppUtilities::DateTime created = CppUtilities::DateTime::gmtNow(); - CppUtilities::DateTime started; - CppUtilities::DateTime finished; - std::vector startAfter; - std::string directory; - std::vector sourceDbs, destinationDbs; - std::vector packageNames; - BuildActionFlagType flags = noBuildActionFlags; - std::unordered_map settings; - std::vector logfiles; std::vector artefacts; std::variant, LibPkg::LicenseResult, LibPkg::PackageUpdates, BuildPreparation, BuildProgress, @@ -251,22 +258,22 @@ private: std::unique_ptr m_internalBuildAction; }; -inline bool BuildAction::isScheduled() const +inline bool BuildActionBase::isScheduled() const { return status == BuildActionStatus::Created || status == BuildActionStatus::AwaitingConfirmation; } -inline bool BuildAction::isExecuting() const +inline bool BuildActionBase::isExecuting() const { return status == BuildActionStatus::Enqueued || status == BuildActionStatus::Running; } -inline bool BuildAction::isDone() const +inline bool BuildActionBase::isDone() const { return status == BuildActionStatus::Finished; } -inline bool BuildAction::hasSucceeded() const +inline bool BuildActionBase::hasSucceeded() const { return isDone() && result == BuildActionResult::Success; } @@ -331,44 +338,6 @@ template inline void BuildAction::appendOutput(CppUtilities:: appendOutput(std::move(CppUtilities::argsToString(CppUtilities::EscapeCodes::formattedPhraseString(phrase), std::forward(args)...))); } -struct LIBREPOMGR_EXPORT BuildActionBasicInfo : public ReflectiveRapidJSON::JsonSerializable { - explicit BuildActionBasicInfo(const BuildAction &buildAction) - : id(buildAction.id) - , taskName(buildAction.taskName) - , templateName(buildAction.templateName) - , directory(buildAction.directory) - , packageNames(buildAction.packageNames) - , sourceDbs(buildAction.sourceDbs) - , destinationDbs(buildAction.destinationDbs) - , startAfter(buildAction.startAfter) - , settings(buildAction.settings) - , flags(buildAction.flags) - , type(buildAction.type) - , status(buildAction.status) - , result(buildAction.result) - , created(buildAction.created) - , started(buildAction.started) - , finished(buildAction.finished) - { - } - - const BuildAction::IdType id; - const std::string &taskName; - const std::string &templateName; - const std::string &directory; - const std::vector &packageNames; - const std::vector &sourceDbs, &destinationDbs; - const std::vector &startAfter; - const std::unordered_map settings; - const BuildActionFlagType flags = noBuildActionFlags; - const BuildActionType type; - const BuildActionStatus status; - const BuildActionResult result; - const CppUtilities::DateTime created; - const CppUtilities::DateTime started; - const CppUtilities::DateTime finished; -}; - } // namespace LibRepoMgr #endif // LIBREPOMGR_BUILD_ACTION_H diff --git a/librepomgr/serversetup.cpp b/librepomgr/serversetup.cpp index 37be830..80e0b22 100644 --- a/librepomgr/serversetup.cpp +++ b/librepomgr/serversetup.cpp @@ -397,14 +397,14 @@ void ServiceSetup::BuildSetup::rebuildDb() } void ServiceSetup::BuildSetup::forEachBuildAction( - std::function count, std::function &&func, std::size_t limit, std::size_t start) + std::function count, ServiceSetup::BuildSetup::BuildActionVisitorBase &&func, std::size_t limit, std::size_t start) { auto txn = m_storage->buildActions.getROTransaction(); const auto total = txn.size(); count(std::min(limit, total)); const auto reverse = start == std::numeric_limits::max(); - for (auto i = reverse ? txn.rbegin() - : txn.lower_bound(static_cast( + for (auto i = reverse ? txn.rbegin() + : txn.lower_bound(static_cast( start > std::numeric_limits::max() ? std::numeric_limits::max() : start)); i != txn.end() && limit; reverse ? --i : ++i, --limit) { if (func(i.getID(), std::move(i.value()))) { @@ -413,7 +413,7 @@ void ServiceSetup::BuildSetup::forEachBuildAction( } } -void ServiceSetup::BuildSetup::forEachBuildAction(std::function &&func) +void ServiceSetup::BuildSetup::forEachBuildAction(ServiceSetup::BuildSetup::BuildActionVisitorWriteable &&func) { auto txn = m_storage->buildActions.getRWTransaction(); for (auto i = txn.begin(); i != txn.end(); ++i) { diff --git a/librepomgr/serversetup.h b/librepomgr/serversetup.h index 4e3969f..571d5f5 100644 --- a/librepomgr/serversetup.h +++ b/librepomgr/serversetup.h @@ -145,9 +145,10 @@ struct LIBREPOMGR_EXPORT ServiceSetup : public LibPkg::Lockable { std::size_t buildActionCount(); std::size_t runningBuildActionCount() const; void rebuildDb(); - void forEachBuildAction(std::function count, std::function &&func, - std::size_t limit, std::size_t start); - void forEachBuildAction(std::function &&func); + using BuildActionVisitorBase = std::function; + void forEachBuildAction(std::function count, BuildActionVisitorBase &&func, std::size_t limit, std::size_t start); + using BuildActionVisitorWriteable = std::function; + void forEachBuildAction(BuildActionVisitorWriteable &&func); std::vector> followUpBuildActions(BuildActionIdType forId); private: diff --git a/librepomgr/tests/webapi.cpp b/librepomgr/tests/webapi.cpp index 8e4d19f..ca3d7ea 100644 --- a/librepomgr/tests/webapi.cpp +++ b/librepomgr/tests/webapi.cpp @@ -267,7 +267,7 @@ void WebAPITests::testPostingBuildActionsFromTask() CPPUNIT_ASSERT_EQUAL_MESSAGE("build actions actually present", 5_st, building.buildActionCount()); CPPUNIT_ASSERT_EQUAL_MESSAGE("build actions not started yet", 0_st, building.runningBuildActionCount()); building.forEachBuildAction([](std::size_t count) { CPPUNIT_ASSERT_EQUAL_MESSAGE("for-each loop returns correct size", 5_st, count); }, - [](LibPkg::StorageID id, BuildAction &&action) { + [](LibPkg::StorageID id, BuildActionBase &&action) { CPPUNIT_ASSERT_EQUAL_MESSAGE(argsToString("build action ", action.id, " not started yet"), BuildActionStatus::Created, action.status); CPPUNIT_ASSERT_EQUAL_MESSAGE(argsToString("build action ", action.id, " has no result yet"), BuildActionResult::None, action.result); CPPUNIT_ASSERT_EQUAL_MESSAGE(argsToString("build action ", action.id, " has task name assigned"), "foobarbaz"s, action.taskName); diff --git a/librepomgr/webapi/routes_buildaction.cpp b/librepomgr/webapi/routes_buildaction.cpp index 7c861c2..4139361 100644 --- a/librepomgr/webapi/routes_buildaction.cpp +++ b/librepomgr/webapi/routes_buildaction.cpp @@ -36,8 +36,8 @@ void getBuildActions(const Params ¶ms, ResponseHandler &&handler) auto buildActionLock = params.setup.building.lockToRead(); params.setup.building.forEachBuildAction( [&array, &jsonDoc](std::size_t count) { array.Reserve(ReflectiveRapidJSON::JsonReflector::rapidJsonSize(count), jsonDoc.GetAllocator()); }, - [&array, &jsonDoc, limit](LibPkg::StorageID, BuildAction &&buildAction) { - ReflectiveRapidJSON::JsonReflector::push(BuildActionBasicInfo(buildAction), array, jsonDoc.GetAllocator()); + [&array, &jsonDoc, limit](LibPkg::StorageID, BuildActionBase &&buildActionBase) { + ReflectiveRapidJSON::JsonReflector::push(buildActionBase, array, jsonDoc.GetAllocator()); return array.Size() >= limit; }, limit, start);